fix(security): ACL bypass + silent CLIENT TRACKING failure on the inline GET path - #457
Merged
Merged
Conversation
…ine GET path
The monoio inline fast path (`try_inline_dispatch`) answers the plain
`*2 $3 GET <key>` shape straight from the shard map, never entering generic
dispatch. Writes were gated on `can_inline_writes` (which folds in
`conn.acl_skip_allowed()`); reads were gated on nothing. That one omission
produced two independent user-visible defects.
1. ACL bypass (security). An authenticated but restricted user read any key
with plain GET — the inline path runs neither the ACL command check nor
the ACL key-pattern check, and logs nothing. Measured on 0.8.4/0.8.5:
user `-@all` GET secret -> "value" (Redis: -NOPERM)
user `+@READ ~app:*` GET outside -> "value" (Redis: -NOPERM)
SET/DEL/MGET/HGET/TTL/EXISTS/... -> -NOPERM (correctly gated)
Not single-shard-only: at `--shards 4` every key hashing to the
connection's own shard leaked (24/160 and 44/160 across a 4-connection
sweep); `--shards 1` leaked 160/160.
2. CLIENT TRACKING answered +OK and then never invalidated. The same path
also skips `tracking::invalidation::track_read_keys`, so a client-side
caching client's own GETs were never registered and nothing invalidated
them — stale reads forever. An identical trial reading via MGET (not
inline-eligible) received the invalidate; via GET it received nothing.
At `--shards 4` delivery depended on whether the key hashed off the
reader's shard, which is also why `mset_invalidates_every_second_arg_key`
was flaky (measured failing 2 of 3 runs on unmodified 0.8.5).
Fixes:
- `blocking.rs`: `try_inline_dispatch{,_loop}` take `can_inline_reads`;
`is_get` now requires it, mirroring `is_set`/`can_inline_writes`.
- `handler_monoio/mod.rs`: `can_inline_reads = acl_skip_allowed() &&
!conn.tracking_state.enabled`. `acl_skip_allowed()` is hoisted and shared
with the write gate, so the hot path pays nothing it did not already pay.
Deliberately NOT gated on the process-global `tracking_active()`: only a
connection's own reads populate its invalidation set, so one caching
client must not push every other connection off the fast path (writes keep
the global gate — a non-tracking writer must invalidate everyone else).
- `command/client.rs`: `CLIENT TRACKING ON BCAST` with no PREFIX registered
nothing, because the handlers only call `register_prefix` inside
`for prefix in &prefixes`. Redis treats prefix-less BCAST as "every key";
`parse_tracking_args` now normalises it to the empty prefix, which
`TrackingTable`'s `key.starts_with(prefix)` match treats as all keys. One
change covers all three handlers.
The tokio handlers were never affected — they gate at handler_single.rs and
handler_sharded/mod.rs. Every CI test job builds tokio, which is why none of
this was visible in CI.
Verification:
- `tests/acl_inline_read_enforcement.rs` (new): deny-all and key-pattern
users at `--shards 1` and `--shards 4`; red first (160/160, 24/160,
160/160, 44/160 leaks), green after.
- `tests/client_tracking_invalidation.rs`: adds inline-GET tracking at
`--shards 1` and BCAST with/without PREFIX; the pre-existing flaky
`mset_invalidates_every_second_arg_key` is now deterministic (4/4).
- `src/server/conn/tests.rs`: unit guard that a GET is refused when
`can_inline_reads` is false, plus the new arg threaded through 16 call
sites.
- Wire diff vs real Redis 8.6.1: identical on all ACL and tracking probes at
both shard counts.
- Fast path preserved, measured via
`moon_dispatch_path_total{path="local_inline"}`: 2000/2000 GETs still
inlined for the unrestricted default user with AND without
`--requirepass`; 0/2000 for restricted and tracking connections, which
still return correct values via generic dispatch.
author: Tin Dang
|
ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing |
|
Warning Review limit reached
Next review available in: 15 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughMonoio inline GET dispatch now checks ACL, transaction, and tracking state before using the fast path. Restricted or tracked reads use generic dispatch. Prefixless tracking broadcasts match all keys. Regression tests cover ACL, transactions, tracking, and shard configurations. ChangesMonoio inline dispatch
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant MonoioHandler
participant InlineDispatcher
participant GenericDispatcher
participant TrackingState
Client->>MonoioHandler: Send GET
MonoioHandler->>TrackingState: Check transaction and tracking state
MonoioHandler->>InlineDispatcher: Check ACL and read eligibility
alt Eligible connection
InlineDispatcher-->>Client: Return inline GET response
else Restricted, tracked, or transactional connection
MonoioHandler->>GenericDispatcher: Dispatch GET generically
GenericDispatcher-->>Client: Queue or register the read
GenericDispatcher-->>Client: Return checked response
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/acl_inline_read_enforcement.rs`:
- Around line 56-109: Update spawn_moon to return Moon directly and fail after
the readiness timeout instead of returning None. Preserve the existing startup
probing, but include useful startup diagnostics when the process fails to become
ready, and update each caller to use the returned Moon without silently exiting
so the security tests always exercise a real server.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d58e491b-0540-4271-9d4f-700f49192def
📒 Files selected for processing (7)
CHANGELOG.mdsrc/command/client.rssrc/server/conn/blocking.rssrc/server/conn/handler_monoio/mod.rssrc/server/conn/tests.rstests/acl_inline_read_enforcement.rstests/client_tracking_invalidation.rs
…ine read path
Found by the new raw-RESP compatibility harness on its first run against a real
redis-server, after this branch's ACL and tracking fixes were already in place.
Same gate, same one-line shape, and the most client-visible of the three.
MULTI -> +OK
GET k -> $1 v Redis: +QUEUED
EXEC -> *0 Redis: *1[$1 v]
MGET k -> +QUEUED control: not inline-eligible, always queued correctly
The inline fast path answers `*2 $3 GET` from the shard map without entering
generic dispatch, so it never observes `conn.in_multi`. `can_inline_writes`
already carried `!conn.in_multi` — which is why `SET` queued correctly all
along — but `can_inline_reads`, added earlier on this branch for the ACL and
tracking gaps, did not.
The consequence is worse than a stale read. The client receives a reply of the
wrong kind for its position in the exchange (a value where the protocol says
`+QUEUED`), and then an `EXEC` that silently omits the read entirely. A
redis-py or go-redis transaction built on MULTI/EXEC gets an empty result set
for an exchange it believes succeeded — no error is raised anywhere.
Reproduced at `--shards 1` and `--shards 4` (a multi-shard connection leaks
only for keys hashing to its own shard, which are the inline-eligible ones).
Pre-existing on main: before this branch `is_get` was ungated entirely.
Fix: `can_inline_reads` now carries `!conn.in_multi`, sharing the condition
with the write gate.
Verification:
- `tests/multi_queues_inline_get.rs` (new): queue-and-exec at `--shards 1` and
`--shards 4`, red first (`got "$2\r\nv0\r\n"`). Plus three controls that pin
what must NOT change — `MGET` still queues, `SET` still queues, and a plain
`GET` outside a transaction still takes the fast path, including immediately
after a completed MULTI.
- Fast path retained, measured via `moon_dispatch_path_total{path=
"local_inline"}`: 2000/2000 GETs still inlined for the default user, with
`--requirepass`, and after a completed transaction — so `in_multi` clears and
the connection returns to the fast path rather than being permanently
demoted.
- Sibling suites still green: 4 ACL + 8 tracking + 5 multi = 17.
- fmt clean; clippy green on both CI configurations.
Monoio-only, like the other two: every CI test job builds tokio, so no existing
job could observe any of them.
author: Tin Dang
Collaborator
Author
Added: a third defect on the same gate (
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/multi_queues_inline_get.rs`:
- Around line 50-100: Update spawn_moon so readiness failure fails the test
instead of returning None and allowing callers to exit successfully. Change its
failure behavior to panic or return a Result, then update every caller to
propagate or expect that failure while preserving the real Moon server startup
and readiness checks.
- Around line 116-127: Update the test helper method Resp::cmd to read and parse
complete RESP frames instead of treating one TcpStream::read result as the full
response. Reuse or add a test-side RESP parser that handles simple strings, bulk
strings, and nested arrays, and return the fully framed reply before MULTI/EXEC
assertions run.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1f4b0a0f-2c7c-4848-9f11-8fc3fda284e7
📒 Files selected for processing (3)
CHANGELOG.mdsrc/server/conn/handler_monoio/mod.rstests/multi_queues_inline_get.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- src/server/conn/handler_monoio/mod.rs
- CHANGELOG.md
Comment on lines
+116
to
+127
| fn cmd(&mut self, args: &[&str]) -> String { | ||
| let mut out = format!("*{}\r\n", args.len()).into_bytes(); | ||
| for a in args { | ||
| out.extend_from_slice(format!("${}\r\n{a}\r\n", a.len()).as_bytes()); | ||
| } | ||
| self.stream.write_all(&out).expect("write"); | ||
| let mut buf = [0u8; 4096]; | ||
| match self.stream.read(&mut buf) { | ||
| Ok(n) => String::from_utf8_lossy(&buf[..n]).into_owned(), | ||
| Err(e) => format!("<read error: {e}>"), | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for an existing framed RESP test helper before adding another parser.
fd -t f -e rs tests src \
| xargs -r rg -n -C 3 'BufReader|read_exact|read_line|RESP|Resp' || trueRepository: pilotspace/moon
Length of output: 153
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the relevant test file and nearby usage.
if [ -f tests/multi_queues_inline_get.rs ]; then
echo "== outline =="
ast-grep outline tests/multi_queues_inline_get.rs || true
echo "== lines 1-180 =="
sed -n '1,180p' tests/multi_queues_inline_get.rs | nl -ba
fi
echo "== all occurrences of Resp::cmd and cmd helpers =="
rg -n -C 4 'Resp::cmd|fn cmd|read_exact|BufReader|RESP|resp' tests src || trueRepository: pilotspace/moon
Length of output: 821
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file existence =="
test -f tests/multi_queues_inline_get.rs && echo "exists" || echo "missing"
echo "== relevant lines =="
sed -n '1,280p' tests/multi_queues_inline_get.rs
echo "== occurrences =="
rg -n -C 4 'Resp::cmd|fn cmd|read_exact|BufReader|RESP|resp' tests src || trueRepository: pilotspace/moon
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== tool availability =="
for tool in sed cat awk python3 git; do
command -v "$tool" >/dev/null 2>&{1} && echo "$tool found" || echo "$tool missing"
done
echo "== locate file =="
find . -path './tests/multi_queues_inline_get.rs' -type f -print || true
echo "== relevant lines =="
sed -n '1,280p' tests/multi_queues_inline_get.rs
echo "== occurrences =="
rg -n -C 4 'Resp::cmd|fn cmd|read_exact|BufReader|RESP|resp' tests src || truernRepository: pilotspace/moon
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file list =="
git ls-files tests/multi_queues_inline_get.rs
echo "== targeted file lines 1-260 =="
sed -n '1,260p' tests/multi_queues_inline_get.rs
echo "== targeted occurrences =="
rg -n -C 3 '\bResp::cmd\b|\bcmd\b\([^)]*\(|read_exact|BufReader|RESP|resp' tests/multi_queues_inline_get.rs || trueRepository: pilotspace/moon
Length of output: 8348
Read complete RESP frames before assertions.
TcpStream::read may return only part of a response. Resp::cmd converts that partial buffer to a complete reply, so nested MULTI/EXEC checks can accept truncated data or fail intermittently. Use a framed RESP reader that reads simple strings, bulk strings, and nested arrays to completion, or reuse a test-side RESP parser.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/multi_queues_inline_get.rs` around lines 116 - 127, Update the test
helper method Resp::cmd to read and parse complete RESP frames instead of
treating one TcpStream::read result as the full response. Reuse or add a
test-side RESP parser that handles simple strings, bulk strings, and nested
arrays, and return the fully framed reply before MULTI/EXEC assertions run.
Review finding on the new ACL suite, applied to all three suites this branch
relies on. `spawn_moon` returned `Option<Moon>` and every caller did
`let Some(m) = spawn_moon(..) else { return }`, so a server that never became
ready turned the test into a no-op that still reported PASS.
That is the wrong failure mode anywhere, and specifically wrong here: these
three suites are the regression guard for a security bypass, a silent
CLIENT TRACKING failure, and a transaction-correctness bug. A guard that can
quietly stop guarding is how the original defects survived two releases.
- `spawn_moon` now returns `Moon` and panics when the readiness deadline
expires. Callers use the server directly; no early return remains.
- The panic carries diagnostics rather than just the symptom: the child's
status (exited with N / still running but never answered PING / unavailable)
and moon's captured stderr. stderr was `Stdio::null()` and is now redirected
to `moon.stderr` in the per-test temp dir. Verified the redirect surfaces a
real cause — spawning with a bad flag yields
"error: unexpected argument '--this-flag-does-not-exist' found"
where the old path printed only "did not become ready".
- The startup probing itself is unchanged: same `common::spawn_listening` bind
handling, same 10s PING poll at 100ms.
- `client_tracking_invalidation.rs`: `moon_binary()` dropped its `Option`,
which was already vestigial — every return path was `Some(..)`.
Validation: acl_inline_read_enforcement 4/4, client_tracking_invalidation 8/8,
multi_queues_inline_get 5/5. fmt clean; clippy green on both CI configurations
and with --all-targets.
author: Tin Dang
…e sufficient Review of this PR raised a case the suite did not cover. `can_inline_reads` is computed ONCE per read-buffer iteration, before `try_inline_dispatch_loop` walks the buffer — so when `MULTI` and the `GET` after it arrive in the SAME write, the gate is evaluated while `conn.in_multi` is still false. Probed directly: the pipelined case is correct, and measurably still correct with the `!conn.in_multi` gate REMOVED. The inline loop bails at the first command that is not a plain GET/SET, so a buffer containing `MULTI` hands its whole remainder to generic dispatch and never re-enters the inline path. The defect only reproduces when `MULTI` and the `GET` arrive in separate reads, which the existing tests already cover. So this test does not guard the gate — it guards the structural property that makes the gate sufficient. Teaching the inline loop to skip past commands it does not recognise, rather than bailing out, would silently reopen the bug for pipelined clients, and only this test would notice. The doc comment says exactly that, including the measurement, because a test whose value is misdescribed is worse than no test. Also asserts the converse: a `GET` BEFORE the `MULTI` in the same write is still served inline, so the fix did not degrade to "disable the fast path whenever a buffer contains MULTI". 6/6 in this suite; ACL 4/4 and tracking 8/8 unaffected. fmt clean, clippy green with --all-targets. author: Tin Dang
TinDang97
added a commit
that referenced
this pull request
Aug 9, 2026
…0-9-client-compat The ADD tracker had drifted a full release train behind the repo: it still reported `v0-6-0-release` as the active milestone while the repo had shipped v0.6.0, v0.7.0, v0.8.0, and v0.8.5. `add.py check` was red on two records. Re-sync: - `shardslice-migration`: retire the RISK-ACCEPTED waiver that expired 2026-08-01. Its condition — the follow-up "cross-shard-read-acceleration (observe)" — was met in v2-performance / v2-2 xshard-read-validation (PRs #177/#178/#179): the C2 reply-side path recovers 38-49% of the cross-shard read penalty and the remainder is the ~10us irreducible hop. Gate raised RISK-ACCEPTED -> PASS, with the retired waiver kept verbatim in `waiver_retired` so the record is auditable rather than erased. Re-gated by direct state edit because `add.py gate` refuses with `tripwire_missing`. That guard is firing on a schema gap, not a finding: NO task in this project carries a tamper snapshot (all 17 predate the tripwire), so every task would trip it identically — `shardslice-migration` is simply the only one that needed its gate rewritten. Recorded as such in the note. - `fts-posting-rank-tf`: `depends_on: ["none"]` was a literal-string typo that `check` correctly read as an unresolvable dependency. Now `[]`. - `v3-3-vector-kv-polish`, `v3-4-kv-correctness`, `v3-5-write-path-durability`, `v0-6-0-release`: four empty shells, scaffolded but never populated, because delivery moved to PR-driven waves and never routed back through ADD. Marked `superseded` (not `done`) with a pointer to the PRs that actually shipped each scope. `add.py milestone-done` refuses a zero-task milestone — "nothing attached -> nothing proven" — and that refusal is right; claiming `done` would launder a gate that was never run. `superseded` is the honest record and only `done` is load-bearing anywhere in the engine. `add.py check`: 2 failed -> 89 passed, 0 failed, 0 warnings. New milestone `v0-9-client-compat` (production stage, now active), from the client/SDK deep review of v0.8.5. The two P0s found by that review ship separately as the v0.8.6 hotfix (PR #457); this milestone covers the remaining ~20 findings — the surface that stops an unmodified redis-py / go-redis / ioredis / monitoring agent from treating Moon as a drop-in. Eight tasks, breadth-first: `client-compat-harness` and `monoio-ci-coverage` have no dependencies and land first, because every other task cites the harness as its verifier and a verifier merged alongside the fix it verifies proves nothing. Then `client-identity-introspection`, `resp3-type-fidelity`, `pubsub-resp3-push`, `cluster-client-bootstrap`, `info-observability`, and `sdk-wire-form-fixes`. The shared decisions are what the review taught: real Redis is the oracle (never Moon's own expectation — the defects were found exactly where Moon tested Moon); RESP2 and RESP3 are both first-class; a command must not change shape by context (standalone vs MULTI vs pipeline); registered implies reachable; and behavior lands on all three dispatch paths, since a check present on two of three is the precise shape of the v0.8.6 P0. 11/11 exit criteria cite a verifier — the milestone is goal auto-ready. author: Tin Dang
TinDang97
added a commit
that referenced
this pull request
Aug 9, 2026
… redis-server
Moon already had a Redis comparison suite. It could not see reply types.
`scripts/test-commands.sh` drives both servers through `redis-cli`, which renders
replies to human-readable text before any assertion runs — `assert_match_ttl`
even does `tr -d '(integer) '` — and `grep -c -- '-3 '` over its 2426 lines
returns 0, so the entire RESP3 surface was never compared at all. Every
assertion in `tests/redis_compat.rs` (968 lines) is Moon against a hand-written
expectation, with no `redis-server` in the loop. Between them, a wrong reply
TYPE was invisible. That is why ~22 type-level defects reached v0.8.5.
This adds a differ that speaks RESP on a raw socket, so the type byte survives
to the assertion:
- compares in a fixed order — TYPE, then SHAPE, then VALUE — and names which
one diverged, because "wrong reply type" and "right type, wrong value" are
different bugs that must not be reported as one
- runs the full {RESP2, RESP3} x {standalone, MULTI/EXEC, pipeline} matrix, so
a reply that changes shape by context is observable. This is not theoretical:
`apply_resp3_conversion` is called from 11 sites across 3 handlers rather
than one choke point, and the matrix immediately proved the EXEC path is one
of the misses.
- normalizes only what is declared, per entry: exact | sorted | type_only |
numeric_tolerance | ignore_value. No global fuzzy match — that would rebuild
the blindness being removed.
- compares errors on the code (first token) and never on message text
- refuses rather than skips: no redis-server is ERR_NO_ORACLE with exit 2. A
differential harness with nothing to differ against would report a green that
means nothing.
Findings on the first full run (oracle redis 8.6.1, moon 0.8.5+, 152
comparisons): 94 pass, 58 waived, 0 unexplained. Every waiver is a real,
reproduced divergence carrying a required reason and the task that owns its
fix, so CI is a ratchet — a NEW divergence fails the job, and `--strict` fails
the moment a waived one is fixed and its waiver goes stale.
Two of those findings are new, neither visible to any existing test:
1. GET inside MULTI is not queued. On the monoio inline fast path it executes
immediately: the client receives `$1 v` where Redis sends `+QUEUED`, and
EXEC then answers `*0` instead of `*1[$1 v]`. `MGET`, which is not
inline-eligible, queues correctly. Root cause is the same gate as the v0.8.6
P0 (PR #457): `can_inline_writes` carries `!conn.in_multi`,
`can_inline_reads` does not. Pre-existing on main and NOT fixed by #457.
`CONFIG GET` inside MULTI is a second, separate instance of the same class.
2. RESP3 conversion is not applied to EXEC inner replies. SMEMBERS is a Set
outside MULTI and a flat Array inside it; ZSCORE is a Double outside and a
Bulk inside. SISMEMBER passes inside MULTI only because two bugs cancel —
it is over-converted to Boolean outside, and unconverted inside.
Both are monoio-only. Every CI test job builds tokio, so neither could have
been caught by the existing matrix — the same structural blind spot that hid
the v0.8.6 ACL bypass.
`--info-manifest` additionally enumerates 34 INFO fields the standard
monitoring stack reads that Moon does not emit (run_id, tcp_port,
uptime_in_seconds, keyspace_hits/misses, evicted_keys, maxmemory*,
instantaneous_ops_per_sec, ...) — a monitoring agent loses those series
silently, which is worse than an error because the dashboard simply goes blank.
Tests are stdlib `unittest`, not pytest: pytest is absent from the moon-dev VM
that runs the self-hosted job, and a PR-gating job must not depend on a package
hand-installed into a runner that gets rebuilt. 33 unit tests (codec,
comparator, all five policies, every reject code) + 19 end-to-end tests against
real servers, all red before the implementation existed.
New CI job `client-compat` builds moon with the DEFAULT (monoio) runtime — the
one clients actually hit — then runs the unit suite, the e2e suite, the strict
diff, and the INFO coverage report, uploading the machine-readable record.
Also scaffolds ADD milestone v0-9-client-compat: this task plus the seven that
consume its output.
author: Tin Dang
TinDang97
added a commit
that referenced
this pull request
Aug 9, 2026
… a real redis-server (#458) Moon already had a Redis comparison suite. It could not see reply types. `scripts/test-commands.sh` drives both servers through `redis-cli`, which renders replies to human-readable text before any assertion runs — `assert_match_ttl` literally does `tr -d '(integer) '` — and `grep -c -- '-3 '` over its 2426 lines returns 0, so the entire RESP3 surface was never compared. Every assertion in `tests/redis_compat.rs` (968 lines) is Moon against a hand-written expectation with no redis-server in the loop. Between them, a wrong reply TYPE was structurally invisible. That is why ~22 type-level defects reached v0.8.5. This adds a differ that speaks RESP on a raw socket, so the type byte survives to the assertion: - compares TYPE, then SHAPE, then VALUE, naming which one diverged — "wrong reply type" and "right type, wrong value" are different bugs - runs the full {RESP2, RESP3} x {standalone, MULTI/EXEC, pipeline} matrix, so a reply that changes shape by context is observable. Not theoretical: `apply_resp3_conversion` is called from 11 sites across 3 handlers rather than one choke point, and the matrix immediately proved the EXEC path is a miss. - normalizes only what is declared per entry (exact | sorted | type_only | numeric_tolerance | ignore_value). No global fuzzy match — that would rebuild the blindness being removed. - compares errors on the code (first token), never on message text - refuses rather than skips: no redis-server is ERR_NO_ORACLE, exit 2 First full run (oracle redis 8.6.1, 152 comparisons): 98 pass, 54 waived, 0 unexplained. Every waiver carries a required reason and names the task that owns its fix, so CI is a ratchet — a NEW divergence fails the job, and `--strict` fails the moment a waived one is fixed and its waiver goes stale. It found two defects nothing else could see. `GET` inside `MULTI` was executed rather than queued (fixed in #457 as a direct result), and RESP3 conversion is skipped for EXEC inner replies — SMEMBERS is a Set outside MULTI and a flat Array inside, ZSCORE is Double outside and Bulk inside, and SISMEMBER passes inside MULTI only because two bugs cancel. `--info-manifest` additionally enumerates 33 INFO fields the standard monitoring stack reads that Moon does not emit; a monitoring agent loses those series silently, which is worse than an error because the dashboard simply goes blank. The stale-waiver mechanism then proved itself unprompted: once #457's fix landed, `--strict` failed on its own with `ERR_STALE_WAIVER: waivers no longer reproduce: multi_get_must_queue, error_wrongtype`. Both are now unwaived permanent guards. Verification: 33 unit tests (codec, comparator, all five policies, every reject code) + 20 end-to-end tests against real servers, all red before implementation. An adversarial refute-read found and fixed two ways the harness could report results it had not earned — the byte-identity invariant was VACUOUS in the MULTI context (`sent` was reconstructed from the same argv for both servers, so it could never fail; now recorded from the socket), and `--info-manifest` blamed Moon for a wrong pin (it checked only Moon; a field the oracle also lacks is now "fix the pin, not moon"). With that in place the pinned list validates itself: 33 findings, 0 bad pins. Tests are stdlib `unittest`, not pytest — pytest is absent from the moon-dev VM that runs the self-hosted job, and a PR-gating job must not depend on a package hand-installed into a runner that gets rebuilt. The new `client-compat` job builds moon with the DEFAULT (monoio) runtime, the one clients actually hit; every existing CI test job builds tokio, which is why all three defects found this week were invisible. Also included: the ADD milestone scaffold for v0-9-client-compat (this task plus the seven that consume its output) and this task's verify record, which files nine evidence-backed spec deltas plus two honesty deltas. Full CI dispatched on this branch before merge: all 8 jobs green, including macOS and Windows, which pull requests normally skip.
TinDang97
added a commit
that referenced
this pull request
Aug 10, 2026
… not the shipped one (#464) * ci(test): run the monoio suite — CI was testing the fallback runtime, not the shipped one Every CI job that EXECUTED tests did so under `--no-default-features --features runtime-tokio,...`. Moon's default feature set is runtime-monoio, and that is what ships on Linux. The result: 26 monoio integration test files and 30 monoio-gated src/ files were unreachable by CI, and the documented local gate in CLAUDE.md ("Local CI Parity", which runs BOTH suites) was strictly stronger than CI itself. That gap is not theoretical. The v0.8.6 inline-GET ACL bypass (#457) was wrong only on the monoio dispatch path and shipped green. The RESP3 type-fidelity work (#463) had to hand-verify one of its two enqueue sites locally, because CI structurally could not see it. Adds `check-monoio`: self-hosted Linux runner (the only place monoio's io_uring driver executes at all), default feature set, `cargo nextest run --profile ci`, its own CARGO_TARGET_DIR, no continue-on-error, MOON_NO_URING deliberately unset. `--profile ci` is load-bearing rather than incidental: the suite has a known load-sensitive flake class, a bare `cargo test` has no retries, and an intermittently-red required job gets disabled -- which is worse than no job, because it still looks like coverage. The existing profile's retries=2 absorbs it while still reporting FLAKY, so the signal survives. Measured on moon-dev (kernel 6.17) before landing: 5145 passed, 1 flaky, 244 skipped, exit 0, 80.3s of test time. VERIFIED BY NEGATIVE CONTROL, not by inspection. A CI-config change can be green and still be worthless, so the claim was tested directly: a deliberate defect injected on `try_inline_dispatch` (cfg(feature = "runtime-monoio"), so tokio cannot reach it) making inline GET answer "$6\r\nBROKEN\r\n": tokio (CI before this change) : multi_queues_inline_get 6 passed <- ships green monoio (the new job) : multi_queues_inline_get 3 FAILED <- caught Reverted immediately; zero residual markers, and the suite back to 6/6. tests/ci_covers_monoio.rs guards the job itself, because the failure mode of CI coverage is silent -- a job that stops running or is switched to the wrong feature set looks exactly like a green build. It fails on a wrong feature set, continue-on-error, a bare `cargo test`, a shared target dir with the tokio job, or removal of tokio coverage. NOTE FOR THE REVIEWER: adding the job makes it RUN, not BLOCK. It must be added to branch protection to gate merges; until then it is advisory. Refs: .add/tasks/monoio-ci-coverage (gate PASS), milestone v0-9-client-compat author: Tin Dang * ci(fix): the monoio job was running io_uring disabled — MOON_NO_URING was a workflow-level global `check-monoio` exists for one reason: to execute the io_uring driver that actually ships on Linux. Its comment said "MOON_NO_URING is deliberately NOT set here", its job block was clean, and `ci_covers_monoio.rs` asserted that block stayed clean. All three were checking the wrong scope. `MOON_NO_URING: "1"` sat in the workflow-level `env:`, which merges into every job — and a job cannot unset an inherited key (an empty value is still a set variable to `env::var_os`). So the one job whose entire premise was io_uring ran with io_uring force-disabled, and had done since the job was written. `monoio_yield_overhead_is_microscopic` reported it: every `cooperative_yield()` fell through `uring_active()` to the `sleep(ZERO)` timer park, 290ms for 200 yields = 1.45ms/yield, against a 100ms budget. That read like a load-sensitive flake on a shared runner. It was not — same binary on moon-dev, the env var as the only delta: io_uring active -> ok, 0.00s MOON_NO_URING=1 -> FAILED, 0.59s Deterministic in both directions. The test was right and the config was wrong. Fix: MOON_NO_URING moves out of workflow-level env onto the jobs that want it — `check` (tokio; the io_uring bridge floods errors under load), `memory-steady- state` (real server on a GitHub-hosted runner where io_uring may be seccomp- restricted), and `client-compat` (kept so its recorded waiver baseline stays comparable; the differ probes wire shapes, not drivers — commented as such, so it is documented rather than silent). Dropped from macOS/Windows/msrv/console/ lint, where it was inherited dead config: no io_uring on those platforms and those jobs execute no Moon. `ci_covers_monoio.rs` gains the assertion it was missing — the workflow-level `env:` block must not define MOON_NO_URING either. Red before the yaml change for exactly that reason, green after; 5/5. Verified on moon-dev with the job's exact fixed config (default features, io_uring live, nextest --profile ci), which no CI run had ever exercised. author: Tin Dang
TinDang97
added a commit
that referenced
this pull request
Aug 12, 2026
…eding a live bug Two findings from the client-compat harness, which is the check that failed on PR #471 — both real, neither visible to the 14 raw-RESP tests in this task's own suite. 1. ROLE executed at QUEUE time inside MULTI. The first cut intercepted ROLE at the connection layer, ahead of the MULTI queueing step, because its answer lives on ConnectionContext.repl_state rather than in the Database that dispatch() receives. So `MULTI; ROLE; EXEC` replied the role array immediately and EXEC then returned `*0`. The damage is worse than a wrong reply: the command silently vanishes from the EXEC array, so every LATER result shifts down one index and a client reads another command's answer as this one's. Redis queues ROLE like any other command. Fixed by answering ROLE from the shared dispatch table instead, reading the process-global replication handle that INFO already uses and that every entry point registers (main.rs, listener.rs, embedded.rs). That is the only placement where a queued ROLE can work at all, since EXEC replays the queue through dispatch() — and it deleted all three per-handler intercepts, so ROLE now lives in exactly two places instead of five. Verified on the wire: `MULTI; ROLE; PING; EXEC` returns `*2` with the role array first and +PONG still last, and a live replica still reports `slave` through the global handle. ci15 pins the alignment; it fails under both prior states (`*0`, and `*1` with an unknown- command error). Nothing in this task's suite exercised a connection-layer command INSIDE a transaction, which is the coverage lesson: a new intercept has to be tested in MULTI as well as standalone, because its POSITION relative to queueing is the thing that can be wrong. 2. The harness's own divergence test needed a live Moon defect to pass. test_a_diverging_entry_exits_one_and_names_the_divergence borrowed a real bug as its fixture, so it FAILED whenever someone FIXED that bug — a test that punishes the fix. Three fixtures had already been burned this way (GET-inside-MULTI #457, SISMEMBER RESP3 #463, and COMMAND COUNT, retired by this very task, which is what turned the PR red). The durable fix was already designed and filed as #461, so it is implemented here rather than rotating to a fourth defect: a test-only `inject_moon_reply` hook fabricates the divergence, with a guard test asserting the shipped manifest never uses one. Proven load-bearing by disabling the hook, which turns the test red. Manifest: the identity_command_count and identity_role waivers are retired as fixed, and COMMAND INFO / COMMAND GETKEYS / RESET are added as live entries. COMMAND INFO carries a new, accurate waiver — its 10-field SHAPE now matches, but acl_categories is thin (@string where Redis says @READ @string @fast), key_specs is empty, and under RESP3 Redis types flags/acl_categories as Sets and key_specs entries as Maps where Moon emits Arrays. Owners recorded. Harness result against redis-server 8.6.1: PASS=181 FAIL=0 WAIVED=19 (was PASS=179 FAIL=8 before this commit). Gates: fmt, clippy (default + tokio/jemalloc, --all-targets), the identity suite (15 scenarios), and both compat suites (34 differ + 20 e2e) green. Note the unreachable-pattern warning caught during this work: (4, b'r') already existed in dispatch_inner for RPOP, so the first ROLE arm was dead code and EXEC still answered unknown command until it was merged into the existing arm. author: Tin Dang
TinDang97
added a commit
that referenced
this pull request
Aug 12, 2026
…#471) * docs(add): close watch-cas-transactions at PASS, record what the scope gate did not prove The code shipped in #470; this closes the task record. The first `gate PASS` was refused with `scope_violation` naming 7020 files, including `.github/workflows/*.yml` and `Cargo.lock` — paths this task never touched. Cause: the scope baseline is a whole-tree byte snapshot taken at the tests->build crossing, with no git awareness. Between that snapshot and the gate, five dependabot PRs merged into main (#420, #417, #346, #358, #359) and target/ was rebuilt, so every unrelated change read as "this task touched it". Rather than re-snapshot quietly, §6 now records: - the task's ACTUAL change set from `git show --name-only 151a185 1c12af0`, every path of which is inside the declared §5 Scope; - the two entries that needed naming instead of glossing — `conn/mod.rs` (one line, `pub mod watch;`, undeclared) and `.add/tooling/add.py` (25 lines of engine fix carried in from an earlier session, genuinely outside §5); - that the re-taken baseline is the post-merge tree, so the scope walk at this gate compares the tree against itself and proves nothing. The evidence that the build stayed in scope is the git file list, not the green gate. Also ticks the §6 verify checklist against evidence already gathered: the stubbed-counter A/B (reverting only next_birth_version turns 4 new tests red, so the green is earned), the 41->1 tokio failure delta traced to a diskfull $TMPDIR rather than to the change, and the 9/9 dispatched matrix including the Windows/macOS/console jobs no PR run executes. Adds .add/.gitignore, which `add.py init` scaffolds but this project never got. Without it the two 40MB scope-snapshot.json sidecars are commit candidates; they are regenerable working state, and the durable scope declaration is the state.json anchor. One 436K sidecar is already tracked from an older task and is left alone. Method delta for the next loop: as built, any task gated after unrelated merges land inherits an unfalsifiable scope violation. The anchor wants to be a git tree-ish, or the walk should skip gitignored paths and diff the merge-base. author: Tin Dang * feat(command): derive COMMAND/ROLE/RESET/HELLO from real server state Moon's client-identity surface answered from compile-time constants instead of from the server. The failures were invisible by eye because redis-cli renders `:0` and `*0` identically as "0", so every assertion here is on raw RESP bytes. COMMAND and COMMAND COUNT each returned the OTHER'S TYPE: bare COMMAND replied `:0` (an Integer where an Array belongs) and COMMAND COUNT replied `*0` (an Array where an Integer belongs). COMMAND INFO/DOCS/LIST/GETKEYS all replied an empty array. A driver that builds its command map at connect time does not read that as "unsupported" — it reads it as a protocol violation. All six now derive from the COMMAND_META phf registry (263 commands), so registering a command is what makes it introspectable and there is no second table to drift. ROLE and RESET were unknown commands. RESET was registered in the metadata table with full flags while dispatch rejected it — the same advertise-then- reject class as WATCH/UNWATCH before v0.8.6 — and a partial RESET existed only inside handler_sharded's subscribe-mode loop, so it worked if you happened to be subscribed on one runtime and nowhere else. RESET's "default state" is taken from restore_migrated_state(None, ..), the same function ConnectionState::new uses, so it cannot drift from what a fresh connection means by default. HELLO contradicted INFO replication on the same connection: hello_acl built `mode`/`standalone` and `role`/`master` as literals, so a replica announced itself a master. Both fields now read ReplicationState/ClusterState. Redis deliberately uses three vocabularies for this one fact (HELLO -> replica, INFO -> slave, ROLE -> slave); that was measured against a live replica pair on redis-server 8.6.1, and it corrected this task's own frozen contract, which had said "slave" everywhere. The test was made stronger rather than matched to the code: ci12 asserts all three surfaces agree, each in its own vocabulary, plus a negative assertion that HELLO no longer claims master. CLIENT INFO / CLIENT LIST reported the literal `laddr=127.0.0.1:0` from inside the format string, so every client on every listener showed port 0. It now carries the real local address. Wired on all three connection handlers plus the inline fast path. ROLE and RESET cannot ride the shared dispatch table — their answers live on ConnectionContext/ReplicationState, not in the Database that dispatch() receives — so each handler needs its own intercept, and handler_single had neither. That handler is reachable only via listener::run_with_shutdown (an in-process tokio API used by a few test suites; main.rs and embedded.rs both route through run_sharded), which is precisely why it had drifted, and why an A/B caught the gap: reverting the fix there left ci12 green. ci14 drives that handler in-process and goes red with `-ERR unknown command 'ROLE'` without it. Tests: 14 raw-RESP scenarios in tests/client_identity_introspection.rs, with parity legs on the monoio and sharded handlers and the inline path. Startup is serialised across the suite's threads — 13 servers initialising data dirs at once left one unable to answer PING inside 30s at ~1 run in 8; a longer timeout would have been slower and still flaky, and a shared OnceLock server would have leaked a live moon past exit because statics are never dropped. 32/32 green at --test-threads=13 after the fix. Found and filed, not folded in: response batches are serialised at flush time using the FINAL protocol version, so a protocol-changing command retro-encodes earlier replies in the same batch (`HELLO 3` alone -> `%7`; `HELLO 3` + `HELLO 2` in one write -> `*14`). Pre-existing — that reproducer touches none of this code — but RESET, which reverts the protocol by contract, adds a second trigger. Filed as ADD task batch-protocol-version-fidelity with the measurement table. MONITOR was split out to monitor-command-feed at freeze: it is a stream rather than a reply, the only item touching the per-command hot path, and the only one exposing other clients' credentials. Gates: cargo fmt --check, clippy (default + tokio/jemalloc, --all-targets), cargo test --release, and the full tokio CI-parity suite all green. author: Tin Dang * fix(command): queue ROLE inside MULTI, and stop the compat harness needing a live bug Two findings from the client-compat harness, which is the check that failed on PR #471 — both real, neither visible to the 14 raw-RESP tests in this task's own suite. 1. ROLE executed at QUEUE time inside MULTI. The first cut intercepted ROLE at the connection layer, ahead of the MULTI queueing step, because its answer lives on ConnectionContext.repl_state rather than in the Database that dispatch() receives. So `MULTI; ROLE; EXEC` replied the role array immediately and EXEC then returned `*0`. The damage is worse than a wrong reply: the command silently vanishes from the EXEC array, so every LATER result shifts down one index and a client reads another command's answer as this one's. Redis queues ROLE like any other command. Fixed by answering ROLE from the shared dispatch table instead, reading the process-global replication handle that INFO already uses and that every entry point registers (main.rs, listener.rs, embedded.rs). That is the only placement where a queued ROLE can work at all, since EXEC replays the queue through dispatch() — and it deleted all three per-handler intercepts, so ROLE now lives in exactly two places instead of five. Verified on the wire: `MULTI; ROLE; PING; EXEC` returns `*2` with the role array first and +PONG still last, and a live replica still reports `slave` through the global handle. ci15 pins the alignment; it fails under both prior states (`*0`, and `*1` with an unknown- command error). Nothing in this task's suite exercised a connection-layer command INSIDE a transaction, which is the coverage lesson: a new intercept has to be tested in MULTI as well as standalone, because its POSITION relative to queueing is the thing that can be wrong. 2. The harness's own divergence test needed a live Moon defect to pass. test_a_diverging_entry_exits_one_and_names_the_divergence borrowed a real bug as its fixture, so it FAILED whenever someone FIXED that bug — a test that punishes the fix. Three fixtures had already been burned this way (GET-inside-MULTI #457, SISMEMBER RESP3 #463, and COMMAND COUNT, retired by this very task, which is what turned the PR red). The durable fix was already designed and filed as #461, so it is implemented here rather than rotating to a fourth defect: a test-only `inject_moon_reply` hook fabricates the divergence, with a guard test asserting the shipped manifest never uses one. Proven load-bearing by disabling the hook, which turns the test red. Manifest: the identity_command_count and identity_role waivers are retired as fixed, and COMMAND INFO / COMMAND GETKEYS / RESET are added as live entries. COMMAND INFO carries a new, accurate waiver — its 10-field SHAPE now matches, but acl_categories is thin (@string where Redis says @READ @string @fast), key_specs is empty, and under RESP3 Redis types flags/acl_categories as Sets and key_specs entries as Maps where Moon emits Arrays. Owners recorded. Harness result against redis-server 8.6.1: PASS=181 FAIL=0 WAIVED=19 (was PASS=179 FAIL=8 before this commit). Gates: fmt, clippy (default + tokio/jemalloc, --all-targets), the identity suite (15 scenarios), and both compat suites (34 differ + 20 e2e) green. Note the unreachable-pattern warning caught during this work: (4, b'r') already existed in dispatch_inner for RPOP, so the first ROLE arm was dead code and EXEC still answered unknown command until it was merged into the existing arm. author: Tin Dang
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.
Summary
The monoio inline fast path (
try_inline_dispatch) answers the plain*2 $3 GET <key>shape straight from the shard map, never entering generic dispatch. Writes were gated oncan_inline_writes(which folds inconn.acl_skip_allowed()); reads were gated on nothing. That single omission produced two independent user-visible defects, one of them a security bypass.Found during a client/SDK compatibility deep review of v0.8.5. Both defects are present on 0.8.4 and 0.8.5 under the default (monoio) runtime.
1. ACL bypass (security)
An authenticated but restricted user reads any key with plain
GET. The inline path runs neither the ACL command check nor the ACL key-pattern check, and logs nothing.Measured against 0.8.5:
Only
GETleaks — every other command routes through generic dispatch and is enforced correctly.Not a single-shard-only quirk. At
--shards 4, every key hashing to the connection's own shard leaked (24/160 and 44/160 across a 4-connection sweep — the ~1/N shard-local fraction). At--shards 1, 160/160.2. CLIENT TRACKING answered
+OKand then never invalidatedThe same path also skips
tracking::invalidation::track_read_keys, so a client-side-caching client's ownGETs were never registered and nothing ever invalidated them — stale reads forever, with no error surfaced.Isolated without a rebuild: an identical trial reading via
MGET(not inline-eligible) received the invalidate; viaGETit received nothing. At--shards 4delivery depended on whether the key hashed off the reader's shard, which is also whymset_invalidates_every_second_arg_keywas flaky — measured failing 2 of 3 runs on unmodified 0.8.5.Separately,
CLIENT TRACKING ON BCASTwith noPREFIXregistered nothing: all three handlers only callregister_prefixinsidefor prefix in &prefixes, so an empty list was a silent no-op. Redis treats prefix-less BCAST as "every key".Changes
src/server/conn/blocking.rs—try_inline_dispatch{,_loop}take a newcan_inline_reads;is_getnow requires it, mirroringis_set/can_inline_writes.src/server/conn/handler_monoio/mod.rs—can_inline_reads = acl_skip_allowed() && !conn.tracking_state.enabled.acl_skip_allowed()is hoisted and shared with the write gate, so the hot path pays nothing it did not already pay (one Acquire load + a compare, previously computed for the write gate alone).Deliberately not gated on the process-global
tracking_active(): only a connection's own reads populate its own invalidation set, so one caching client must not push every other connection off the fast path. Writes keep the global gate — a non-tracking writer must still invalidate everyone else.src/command/client.rs—parse_tracking_argsnormalises prefix-less BCAST to the empty prefix, whichTrackingTable'skey.starts_with(prefix)match treats as all keys. One change covers all three handlers.The tokio handlers were never affected — they gate at
handler_single.rsandhandler_sharded/mod.rs.Why CI did not catch this
Every CI test job builds tokio. The defect is monoio-only, so no existing job could have observed it. This is a structural blind spot, not a coverage gap in any one test — filed as a follow-up (see below).
The new tests are written so they pass trivially under tokio and actually exercise the bug under monoio; both are documented in the test files.
Verification
tests/acl_inline_read_enforcement.rs(new) — deny-all and key-pattern users at--shards 1and--shards 4. Red first (160/160, 24/160, 160/160, 44/160 leaks), green after.tests/client_tracking_invalidation.rs— adds inline-GET tracking at--shards 1plus BCAST with and without PREFIX. The pre-existing flakymset_invalidates_every_second_arg_keyis now deterministic (4/4).src/server/conn/tests.rs— unit guard that a GET is refused whencan_inline_readsis false (zero bytes consumed, buffer untouched), plus the new arg threaded through 16 call sites.ab4c393e(monoio_yield_overhead_is_microscopic— pre-existing, filed separately; and one load flake that clears in isolation).runtime-tokio,jemalloc): green.Fast path preserved
Measured directly via
moon_dispatch_path_total{path="local_inline"}rather than inferred from latency — this proves identical path selection, not just similar timing:--requirepassLinux A/B on the VM, one server alive at a time, alternating order, 5 samples each, medians: c=1 P=1 +4.95% (within run-to-run noise — the base column itself swings wider than that), c=8 P=16 0.00%.
Follow-ups (not in this PR)
monoio_yield_overhead_is_microscopicfails onmain(578ms for 200 yields vs a <100ms bound) — the v2-perfcooperative_yieldself-pipe fix appears to have regressed to timer-park cost. Invisible because it is monoio-only.cargo clippy --all-targetsfails on pre-existing lints intests/busy_poll_idle.rsandsrc/io/fd_table.rs; CI omits--all-targets, so test-code lints never gate.The remaining ~20 client-compat findings from the review (RESP3 type fidelity, HELLO identity, pub/sub push frames, cluster bootstrap, INFO telemetry) are scoped as a separate v0.9 milestone.
Summary by CodeRabbit
Security
Bug Fixes
CLIENT TRACKING ON BCASTwithout a prefix now tracks all keys.GETcommands insideMULTIare correctly queued untilEXEC.