diff --git a/.add/state.json b/.add/state.json
index 546fd244..e8807d9d 100644
--- a/.add/state.json
+++ b/.add/state.json
@@ -1,7 +1,7 @@
{
"project": "moon",
"stage": "production",
- "active_task": "cluster-client-bootstrap",
+ "active_task": "batch-protocol-version-fidelity",
"active_milestone": "v0-9-client-compat",
"tasks": {
"hotpath-lock-quickwins": {
@@ -283,14 +283,14 @@
},
"cluster-client-bootstrap": {
"title": "CLUSTER SHARDS, READONLY/READWRITE, and honest cluster_state",
- "phase": "build",
- "gate": "none",
+ "phase": "done",
+ "gate": "PASS",
"milestone": "v0-9-client-compat",
"depends_on": [
"client-compat-harness"
],
"created": "2026-08-09T07:32:04+00:00",
- "updated": "2026-08-14T12:42:38+00:00",
+ "updated": "2026-08-14T21:50:36+00:00",
"flag_verified": true,
"tripwire": {
"contract_md5": "f2641c3d2ef1f5db4f16908ddda5d6ef",
@@ -305,11 +305,12 @@
"src/acl/rules.rs",
"tests/cluster_client_bootstrap.rs",
"tests/cluster_formation.rs",
+ "tests/integration.rs",
"scripts/client-compat/manifest.yaml",
"CHANGELOG.md",
"tmp/"
],
- "snapshot_md5": "ca6f6bc81873a042b9e501878a963083"
+ "snapshot_md5": "da2374d445e1db4810604b926e214e07"
}
},
"info-observability": {
@@ -484,12 +485,25 @@
},
"batch-protocol-version-fidelity": {
"title": "Response batch must be encoded in the protocol in effect when each reply was produced",
- "phase": "ground",
- "gate": "none",
+ "phase": "done",
+ "gate": "PASS",
"milestone": "v0-9-client-compat",
"depends_on": [],
"created": "2026-08-11T17:33:52+00:00",
- "updated": "2026-08-11T17:33:52+00:00"
+ "updated": "2026-08-14T22:25:41+00:00",
+ "flag_verified": true,
+ "tripwire": {
+ "contract_md5": "c674b96cc2e31636dd0bdd03aa3408be",
+ "tests": {}
+ },
+ "scope": {
+ "declared": [
+ "src/server/conn/core.rs",
+ "src/server/conn/shared.rs",
+ "src/server/conn/handler_monoio/"
+ ],
+ "snapshot_md5": "c4d153dadf22e877b4096bf8f937812b"
+ }
}
},
"milestones": {
@@ -595,7 +609,7 @@
}
},
"created": "2026-06-11T03:18:21+00:00",
- "updated": "2026-08-14T12:42:38+00:00",
+ "updated": "2026-08-14T22:25:41+00:00",
"setup": {
"locked": true,
"locked_at": "2026-06-11T03:28:00+00:00",
diff --git a/.add/tasks/batch-protocol-version-fidelity/TASK.md b/.add/tasks/batch-protocol-version-fidelity/TASK.md
index 668b3d73..dedbb310 100644
--- a/.add/tasks/batch-protocol-version-fidelity/TASK.md
+++ b/.add/tasks/batch-protocol-version-fidelity/TASK.md
@@ -2,7 +2,7 @@
slug: batch-protocol-version-fidelity · created: 2026-08-12 · stage: production
autonomy: auto
-phase: ground
+phase: done
@@ -15,10 +15,30 @@ phase: ground
+
---
@@ -89,11 +150,43 @@ Assumptions — lowest-confidence first:
```gherkin
-Scenario:
- Given
- When
- Then
- And # required for every rejection
+Scenario: a reply produced before HELLO 2 keeps its RESP3 encoding
+ Given a connection that has completed HELLO 3
+ When it sends CONFIG GET maxmemory / HELLO 2 / CONFIG GET maxmemory in ONE write
+ Then the first reply is a RESP3 map (%)
+ And the HELLO reply is the RESP2 array (*) that HELLO 2 itself establishes
+ And the third reply is a RESP2 array (*)
+
+Scenario: a reply produced before HELLO 3 keeps its RESP2 encoding
+ Given a connection still on RESP2
+ When it sends CONFIG GET maxmemory / HELLO 3 / CONFIG GET maxmemory in ONE write
+ Then the first reply is a RESP2 array (*)
+ And the HELLO reply is a RESP3 map (%)
+ And the third reply is a RESP3 map (%)
+
+Scenario: two HELLOs in one batch each take effect from their own index
+ Given a connection still on RESP2
+ When it sends CONFIG GET / HELLO 3 / CONFIG GET / HELLO 2 / CONFIG GET in ONE write
+ Then the type bytes are, in order: * % % * *
+ And no reply is encoded under a version that took effect after it was produced
+
+Scenario: a batch without HELLO is encoded entirely in one protocol
+ Given a connection that has completed HELLO 3
+ When it sends five CONFIG GETs in ONE write
+ Then every reply is a RESP3 map (%)
+ And the no-switch fast path is what produced them (nothing else changed)
+
+Scenario: CONFIG GET honours every parameter, not just the first
+ Given any connection
+ When it sends CONFIG GET maxmemory appendonly
+ Then the reply names both maxmemory and appendonly
+ And an unknown pattern in the same call is silently skipped, not an error
+
+Scenario: CONFIG GET deduplicates overlapping patterns
+ Given any connection
+ When it sends CONFIG GET maxmemory maxmemory*
+ Then maxmemory is reported exactly once
+ And the surviving entries keep the server's own table order
```
@@ -105,36 +198,79 @@ Scenario:
## 3 · CONTRACT — freeze the shape ▸ docs/05-step-3-contract.md
```
- body: { }
- 200 -> { }
- 4xx -> { error: "" | "" }
-Schema:
+Wire contract (RESP, not HTTP) — per CONNECTION, per BATCH:
+
+ For a batch of replies R[0..n) flushed in one write, let V(i) be the connection's
+ protocol_version at the moment R[i] was PRODUCED. Then R[i] is serialized with
+ serialize_resp3 iff V(i) >= 3, else with serialize.
+
+ V(0) = the version in effect when the batch began.
+ A protocol-changing command at index k sets V(i) = new for all i >= k (INCLUSIVE of k:
+ a HELLO's own reply is encoded in the protocol that HELLO establishes — measured on
+ redis-server 8.6.1).
+
+Internal shape (frozen):
+ ConnectionState {
+ proto_switches: SmallVec<[(usize, u8); 2]>, // (reply index, version) in batch order
+ proto_batch_start: u8, // version V(0) for the pending batch
+ }
+ shared::note_protocol_switch(conn, at: usize, version: u8)
+ -> MUST be called BEFORE `conn.protocol_version` is reassigned; it reads the old value to
+ learn what the batch STARTED in. Called with `at = responses.len()`.
+ shared::encode_response_batch(conn, responses: &[Frame], buf: &mut BytesMut)
+ -> encodes per the rule above and CLEARS proto_switches. With proto_switches empty it is
+ the previous single-version loop: one branch, no allocation.
+
+ CONFIG GET
+ -> flat array, the UNION over patterns, deduplicated, in the server's own table order;
+ unknown patterns contribute nothing (all-unknown -> empty array)
+ -> non-string argument: -ERR invalid argument
```
-Status: DRAFT
-
+Status: FROZEN @ v1 — approved by Tin Dang (auto, `autonomy: auto`)
+
+Least-sure flag surfaced at freeze: **[contract] the INCLUSIVE boundary** — whether a HELLO's
+own reply belongs to the old protocol or the new one. Cost if wrong: the handshake reply itself
+is misparsed, which is worse than the bug being fixed. Resolved by measurement against
+redis-server 8.6.1 rather than by reasoning: `HELLO 3` on a RESP2 connection answers `%7`, so the
+switch is inclusive of its own index. `note_protocol_switch(conn, responses.len(), …)` is called
+BEFORE the HELLO reply is pushed, which is what makes the recorded index inclusive.
---
## 4 · TESTS — failing-first suite (red) ▸ docs/06-step-4-tests.md
-Coverage target:
+Coverage target: every Must and every Reject above has a test; both runtimes; shards 1 and 4.
+
Plan (one test per scenario, asserting behavior not internals):
- - test_: arrange / act / assert + assert
+ - bpv1_a_reply_produced_before_hello_2_keeps_its_resp3_encoding: arrange HELLO 3 / act one
+ pipelined write CONFIG GET, HELLO 2, CONFIG GET / assert type bytes "%**" (RED before fix:
+ observed "***")
+ - bpv2_a_reply_produced_before_hello_3_keeps_its_resp2_encoding: assert "*%%" — the direction
+ that already passes, PINNED so the fix cannot trade one direction for the other
+ - bpv3_two_hellos_in_one_batch_each_take_effect_from_their_own_index: assert "*%%**"
+ (RED before fix: observed "*****")
+ - bpv4_a_batch_without_hello_is_encoded_entirely_in_one_protocol: assert "%%%%%" — pins the
+ no-switch fast path against regression
+ - bpv5_config_get_honours_every_parameter_not_just_the_first: assert both names present, an
+ unknown pattern skipped rather than erroring (RED before fix: second name absent)
+ - bpv6_config_get_deduplicates_overlapping_patterns: assert maxmemory appears exactly once
+ - bpv7_reset_is_a_protocol_switch_and_does_not_reach_backwards: assert "%+*" — RESET is the
+ SECOND protocol-moving command and fails through a different code path
+ (`shared::try_handle_reset`), so a HELLO-only fix leaves it red (RED at the time it was added,
+ AFTER the HELLO sites were already green: observed "*+*")
+ - shared.rs::proto_walk_tests (5 unit tests): a switch never reaches backwards; a switch applies
+ at its OWN index; every switch is honoured in order; no switches = one version throughout; a
+ switch beyond the last reply is inert
-Tests live in: `./tests/` · MUST run red (missing implementation) before Build.
-
+Tests live in: `tests/batch_protocol_version.rs` · `src/server/conn/shared.rs`
+MUST run red (missing implementation) before Build — confirmed: bpv1, bpv3, bpv5 red; bpv2, bpv4,
+bpv6 green from the start (they are pins, not proofs).
+
+Every test body runs against a server on `--shards 1` AND `--shards 4` (`on_each_shard_count`),
+because this repo's recurring defect class is a behaviour present on some dispatch paths only.
@@ -142,64 +278,137 @@ Tests live in: `./tests/` · MUST run red (missing implementation) before Build.
## 5 · BUILD — AI writes code ▸ docs/07-step-5-build.md
-Scope (may touch): `./src/`
-Strategy (ordered batches): <1. … 2. … — the planned build order; guidance, not enforced>
-Safety rule (feature-specific):
-Code lives in: `./src/`
-Constraints: do NOT change any test or the contract; allow-list packages only; ask if unclear.
+Scope (may touch): `src/server/conn/core.rs` `shared.rs` `src/server/conn/handler_monoio/`
+`src/server/conn/handler_sharded/` `src/command/config.rs` `tests/batch_protocol_version.rs`
+
+Strategy (ordered batches):
+ 1. Write the red suite against the CURRENT binary; record which tests are red and which are pins.
+ 2. Add `proto_switches` + `proto_batch_start` to `ConnectionState` and the two helpers
+ (`note_protocol_switch`, `encode_response_batch`) + `ProtoWalk` unit tests in `shared.rs`.
+ 3. Wire the four monoio sites (2 HELLO, 2 flush), then the four sharded sites.
+ 4. Fix `config_get` to iterate every pattern.
+ 5. Prove non-vacuity by reverting the walk to a single version and confirming exactly bpv1 and
+ bpv3 fail.
+
+Safety rule (feature-specific): `note_protocol_switch` MUST run before `conn.protocol_version` is
+reassigned. Reversed, it records the NEW version as the batch start and the fix silently becomes a
+no-op for the first switch — this exact ordering bug occurred during build and was caught by bpv1.
-
+Code lives in: `src/server/conn/`, `src/command/config.rs`
+Constraints: do NOT change any test or the contract; no allocation added to a switch-free batch.
---
## 6 · VERIFY — evidence + non-functional review ▸ docs/08-step-6-verify.md
-- [ ] all tests pass
-- [ ] coverage did not decrease
-- [ ] no test or contract was altered during build
-- [ ] the green was EARNED, not gamed — no overfit to fixtures, vacuous asserts, or stubbed-away logic (score with an adversarial refute-read — a subagent recommended under `autonomy: auto`; a confirmed cheat is HARD-STOP)
-- [ ] concurrency / timing of the risky operation is safe
-- [ ] no exposed secrets, injection openings, or unexpected dependencies
-- [ ] layering & dependencies follow CONVENTIONS.md
-- [ ] a person reviewed and approved the change
-
-### Build expectations — what "correct" looks like (fill BEFORE build; confirm each at the gate)
-> Pre-declare the OBSERVABLE outcomes a correct build must produce — derived from §2 SCENARIOS
-> + §3 CONTRACT — so this gate checks the build is RIGHT, not merely that tests are green. Each
-> row is evidence you can SEE, not a restatement of a test name.
-- [ ] — confirmed by
-- [ ] — confirmed by
-
-### Deep checks — do not skim (fill the path that applies; the resolver judges which)
-- [ ] WIRING (code) — every new symbol is referenced; record where / how confirmed
-- [ ] DEAD-CODE (code) — no new unused or orphaned symbol introduced
-- [ ] SEMANTIC (prose / non-code) — read in full, not skimmed:
+- [x] all tests pass — monoio 6/6 + tokio 6/6 (`batch_protocol_version`); `proto_walk_tests` 5/5;
+ lib 4640 (monoio) / 3806 (tokio); integration 108; `resp3_hello` 1, `resp3_type_fidelity` 13,
+ `pubsub_resp3_push` 21, `protocol_error_lifetime` 8 — all green
+- [x] coverage did not decrease — 11 tests added, none removed or weakened
+- [x] no test or contract was altered during build
+- [x] the green was EARNED — revert probe: with `ProtoWalk::new(conn.protocol_version, &[])`
+ (i.e. the old single-version behaviour) EXACTLY bpv1 and bpv3 fail and nothing else does.
+ The suite is therefore neither vacuous nor overfit: it fails for the defect and only for it.
+- [x] concurrency / timing — the new state is per-connection and touched only on the connection's
+ own task; no lock, no shared mutation, no `.await` held across it
+- [x] no exposed secrets, injection openings, or unexpected dependencies — `smallvec` was already
+ a direct dependency; nothing else added
+- [x] layering & dependencies follow CONVENTIONS.md — the shared logic lives in
+ `server/conn/shared.rs`, which is exactly the module both handlers already share; no
+ handler-to-handler dependency introduced
+- [x] a person reviewed and approved the change — Tin Dang, standing approval for this milestone
+
+### Build expectations — what "correct" looks like
+- [x] A RESP3 connection pipelining `CONFIG GET / HELLO 2 / CONFIG GET` reads `%`, `*`, `*` —
+ confirmed on the wire by `bpv1` and by hand against live redis-server 8.6.1, which answers
+ byte-for-byte the same TYPES for all four probe cases (A up, B down, C many, D dbl).
+- [x] A batch containing no HELLO allocates nothing new — confirmed by reading
+ `encode_response_batch`: the `proto_switches.is_empty()` arm returns before `ProtoWalk` is
+ constructed, and `SmallVec::new()` is inline-capacity, never heap, until a switch is pushed.
+- [x] Both shipped handlers agree — confirmed by running the whole suite at `--shards 1` and
+ `--shards 4` on both runtime legs (4 combinations, 24 test runs).
+- [x] `CONFIG GET maxmemory appendonly` names both — confirmed by `bpv5`; ordering/dedup semantics
+ measured against redis-server 8.6.1 rather than assumed.
+
+### Deep checks
+- [x] WIRING (code) — `note_protocol_switch` referenced at 4 sites (monoio dispatch.rs ×2,
+ handler_sharded/mod.rs ×2); `encode_response_batch` referenced at 4 flush sites
+ (handler_monoio/mod.rs ×2, handler_sharded/mod.rs ×2); `proto_switches` /
+ `proto_batch_start` read only through those two helpers. Confirmed by grep + by the revert
+ probe failing (dead code cannot fail a test).
+- [x] DEAD-CODE — no new unused symbol; `cargo clippy --all-targets -- -D warnings` clean on BOTH
+ feature sets (default/monoio and `runtime-tokio,jemalloc`), which is what caught the one
+ genuinely unused import (`BytesMut`) during the gate.
+
+### CORRECTION to §0
+§0 recorded the second reproducing path as "the tokio `handler_single` path". That is imprecise:
+`main.rs` drives `run_sharded` at both call sites, so the binary never reaches `handler_single`.
+The two SHIPPED paths are `handler_monoio` and `handler_sharded`, and both are fixed. The §0
+measurement itself stands — the defect reproduced on both runtime legs, as the tokio leg of the
+suite still confirms.
+
+### SECOND SWITCH FOUND AT THE GATE — `RESET`
+The first green suite covered only `HELLO`. A wiring sweep for every writer of
+`conn.protocol_version` (`grep -rn '\.protocol_version = '`, 6 sites) surfaced a third:
+`shared::try_handle_reset`, which restores the connection's default state — RESP2 included. §0 had
+already MEASURED `HELLO 3` + `RESET` producing `*14`, so this was a gap between the recorded
+evidence and the fix, not a new discovery. `bpv7` was written against the already-fixed binary,
+observed red (`*+*`, want `%+*`), and went green once `try_handle_reset` records its switch. The
+lesson generalises: "which command changes the protocol" is a set, not a special case, and the
+sweep that finds all of them is over the ASSIGNMENTS, not over the command names.
+
+### KNOWN LIMITATION — `handler_single`
+`handler_single` (the library/embedding path, `server::run`; `main.rs` drives `run_sharded` at both
+call sites, so no shipped binary reaches it) flushes through `Framed::send`, encoding each frame
+with the codec's version at send time, and therefore still retro-encodes a batch containing a
+protocol switch. It is NOT fixed here: its two flush sites include `flush_with_aof_ack`, which
+takes a bare sink and no `ConnectionState`, so threading the switch walk through it is a change of
+a different size on a path with no shipped caller. What IS done is bounding it — the handler clears
+`proto_switches` at each batch boundary, because it shares `try_handle_reset` and would otherwise
+accumulate switch records forever on a connection that RESETs repeatedly. Filed as a spec delta.
### GATE RECORD
-Outcome:
-If RISK-ACCEPTED -> owner: · ticket: · expires: (never for a security gap)
-Reviewed by: · date:
-
-
+Outcome: PASS
+Reviewed by: Tin Dang · date: 2026-08-15
---
## 7 · OBSERVE — feed the next loop ▸ docs/09-the-loop.md
-Watch (reuse scenarios as monitors):
+Watch (reuse scenarios as monitors): the four bpv wire assertions run on every CI leg and at
+shards 1 and 4 — a regression shows up as a type-byte mismatch, not as a latency number. The
+no-switch fast path is the thing most likely to be quietly undone by a future refactor, which is
+why bpv4 pins it even though it has never been red.
### Spec delta
Forward changes for the next loop — each re-enters at Specify as the next task. One line
each, tagged `[SPEC · open|seeded|dropped]`, with evidence (e.g. `[SPEC · open] rate-limit
the retry path (evidence: prod herd spikes)`). See the `add` skill's `deltas.md`.
+ - [SPEC · open] any FUTURE command that mutates `conn.protocol_version` must call
+ `note_protocol_switch` before the assignment; nothing enforces this today (evidence: the
+ ordering bug occurred during this very build and was caught only by bpv1, not by the compiler)
+ - [SPEC · open] `handler_single` still retro-encodes a batch containing a protocol switch; it is
+ bounded (switch record cleared per batch) but not fixed, because `flush_with_aof_ack` takes a
+ sink with no `ConnectionState` (evidence: the KNOWN LIMITATION section above; no shipped binary
+ reaches this handler, which is why it was scoped out rather than rushed)
+ - [SPEC · open] `CONFIG GET` reports a hand-maintained table of ~23 parameters; anything Moon
+ accepts via `CONFIG SET` but omits here is invisible to a client that reads its own config
+ back (evidence: found while measuring dedup/order semantics against redis-server 8.6.1)
+
### Competency deltas
What did this loop teach the foundation? One line each, tagged by competency
(`DDD · SDD · UDD · TDD · ADD`), status `open`, with evidence. See the `add` skill's `deltas.md`.
+
+ - [TDD · open] a test that passes BEFORE the fix is still worth writing, but must be labelled a
+ pin rather than counted as red — bpv2/bpv4/bpv6 were green from the start, and calling the
+ suite "red" without that distinction would have overstated the evidence (evidence: only
+ bpv1/bpv3/bpv5 were genuinely red; the revert probe confirms exactly bpv1+bpv3 own the fix)
+ - [ADD · open] a defect reachable only by a single `write()` of two commands is invisible to
+ every `redis-cli`-driven and client-library-driven test in this repo; wire-level suites need a
+ raw-socket harness, not a client (evidence: this bug survived 13 prior milestone tasks)
+ - [TDD · open] when a defect has a KNOWN second trigger recorded in §0, write its test even after
+ the first one is green — the RESET case was measured in §0, survived the whole build, and was
+ caught only by a gate-time sweep over the state assignment rather than over the command names
+ (evidence: bpv7 red against an otherwise-green binary)
diff --git a/.add/tasks/cluster-client-bootstrap/TASK.md b/.add/tasks/cluster-client-bootstrap/TASK.md
index 742041a8..dcbe6422 100644
--- a/.add/tasks/cluster-client-bootstrap/TASK.md
+++ b/.add/tasks/cluster-client-bootstrap/TASK.md
@@ -2,7 +2,7 @@
slug: cluster-client-bootstrap · created: 2026-08-09 · stage: production
autonomy: auto
-phase: build
+phase: done
@@ -501,7 +501,7 @@ Tests live in: `tests/cluster_client_bootstrap.rs`
## 5 · BUILD — AI writes code ▸ docs/07-step-5-build.md
-Scope (may touch): `src/cluster/` `src/command/connection.rs` `src/command/metadata.rs` `src/server/conn/` `src/acl/rules.rs` `tests/cluster_client_bootstrap.rs` `tests/cluster_formation.rs` `scripts/client-compat/manifest.yaml` `src/../CHANGELOG.md` `tmp/`
+Scope (may touch): `src/cluster/` `src/command/connection.rs` `src/command/metadata.rs` `src/server/conn/` `src/acl/rules.rs` `tests/cluster_client_bootstrap.rs` `tests/cluster_formation.rs` `tests/integration.rs` `scripts/client-compat/manifest.yaml` `src/../CHANGELOG.md` `tmp/`
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 89e54c22..8f05a0e5 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -86,6 +86,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
`cluster-ctl` thread, which by policy aborts the whole server. A short v3 header is now rejected
as malformed. Seeded into `fuzz/corpus/gossip_deser` — the target was correct but had not
synthesised the 4-byte magic plus that 40-byte length window within its PR budget.
+- **A pipelined `HELLO` no longer re-encodes the replies that came before it.** Moon accumulates a
+ read batch's replies and serialized them all at flush time under whichever protocol version was in
+ effect at the END of the batch, so a `HELLO 2` sent in the same write as an earlier command
+ retro-downgraded that earlier reply: `CONFIG GET maxmemory` produced under RESP3 went out as `*2`
+ instead of `%1`. Every reply is now encoded in the protocol that was in effect when that reply was
+ produced, with a switch taking effect from its own index onward — inclusive, so a `HELLO`'s own
+ reply is rendered in the protocol it establishes, which is what redis-server 8.6.1 does. Only the
+ downgrade direction was ever visible: the frame *shape* is already fixed correctly at dispatch, and
+ a RESP2-flattened array re-serialized as RESP3 still emits `*`, which is why the upgrade direction
+ looked fine by accident. It is now pinned in both directions. `RESET` is covered too: it is
+ contracted to return the connection to its default state, RESP2 included, so it moves the protocol
+ exactly as a pipelined `HELLO 2` does — and a fix that covered only the two `HELLO` sites left
+ `HELLO 3` + `RESET` in one write still retro-downgrading. `redis-cli` cannot express two commands
+ in one `write()`, which is how this survived — the new suite drives a raw socket, and runs on both
+ runtimes at 1 and 4 shards. Batches without a protocol switch — essentially all of them — keep the
+ previous single-version loop, one branch and no allocation.
+- **`CONFIG GET` answers every parameter, not just the first.** `CONFIG GET maxmemory appendonly`
+ reported only `maxmemory`; the rest were silently dropped, which is what `redis-py`'s
+ `config_get(*params)` and monitoring agents that read several settings per call send. The reply is
+ now the union over all patterns, deduplicated (`maxmemory` plus `maxmemory*` reports it once), in
+ the server's own table order rather than the caller's argument order, with unknown patterns skipped
+ rather than erroring — all four properties measured against redis-server 8.6.1.
- **`CLUSTER INFO` no longer claims `cluster_enabled`, and a slotless node no longer claims health.**
Two integration assertions encoded the pre-fix behaviour and contradicted the measured oracle:
redis-server 8.6.1 reports `cluster_enabled` in `INFO` only — `CLUSTER INFO` never carries it —
diff --git a/src/command/config.rs b/src/command/config.rs
index 0b95dbf4..d96873e6 100644
--- a/src/command/config.rs
+++ b/src/command/config.rs
@@ -1,4 +1,5 @@
use bytes::Bytes;
+use smallvec::SmallVec;
use crate::command::key::glob_match;
use crate::config::{RuntimeConfig, ServerConfig};
@@ -16,13 +17,26 @@ pub fn config_get(
));
}
- let pattern = match &args[0] {
- Frame::BulkString(s) => s.to_ascii_lowercase(),
- Frame::SimpleString(s) => s.to_ascii_lowercase(),
- _ => {
- return Frame::Error(Bytes::from_static(b"ERR invalid argument"));
+ // Redis accepts MANY parameters, not one: `CONFIG GET maxmemory appendonly`
+ // answers both. Reading only `args[0]` silently dropped the rest, which is
+ // what `redis-py`'s `config_get(*params)` and every monitoring agent that
+ // reads two settings in one call send.
+ //
+ // Measured on redis-server 8.6.1: the result is the UNION over the
+ // patterns, deduplicated (`maxmemory` + `maxmemory*` reports `maxmemory`
+ // once), in the server's own table order rather than the caller's argument
+ // order, with unknown patterns silently skipped. Filtering the table once
+ // per entry — rather than looping the patterns outermost — gives all three
+ // properties for free.
+ let mut patterns: SmallVec<[Vec; 4]> = SmallVec::new();
+ for arg in args {
+ match arg {
+ Frame::BulkString(s) | Frame::SimpleString(s) => patterns.push(s.to_ascii_lowercase()),
+ _ => {
+ return Frame::Error(Bytes::from_static(b"ERR invalid argument"));
+ }
}
- };
+ }
// Build list of all known config parameters
let params: Vec<(&[u8], String)> = vec![
@@ -80,7 +94,7 @@ pub fn config_get(
let mut result = Vec::new();
for (name, value) in params {
- if glob_match(&pattern, name) {
+ if patterns.iter().any(|p| glob_match(p, name)) {
result.push(Frame::BulkString(Bytes::copy_from_slice(name)));
result.push(Frame::BulkString(Bytes::from(value)));
}
diff --git a/src/server/conn/core.rs b/src/server/conn/core.rs
index 727f442b..85523900 100644
--- a/src/server/conn/core.rs
+++ b/src/server/conn/core.rs
@@ -12,6 +12,7 @@
use bytes::Bytes;
use ringbuf::HeapProd;
+use smallvec::SmallVec;
use std::cell::RefCell;
use std::collections::HashMap;
use std::rc::Rc;
@@ -225,6 +226,24 @@ pub(crate) struct ConnectionState {
/// asymmetry is the whole point of the verb, and a "just return +OK"
/// implementation is what gets it wrong.
pub readonly: bool,
+ /// Protocol switch points inside the batch currently being built:
+ /// `(response_index, version_from_that_index_on)`.
+ ///
+ /// A pipelined `HELLO` changes the protocol for the replies AFTER it and
+ /// for its own reply, never for the ones already produced — but a batch is
+ /// serialized in ONE pass at flush, by which time `protocol_version` has
+ /// already moved on. Without these points the batch's final version
+ /// retro-encodes every earlier reply, and a `Frame::Map` produced under
+ /// RESP3 goes out flattened as a RESP2 array.
+ ///
+ /// Empty for every batch containing no `HELLO` — essentially all of them —
+ /// and [`encode_response_batch`] branches on that, so the hot path keeps
+ /// its single-version loop and allocates nothing.
+ pub proto_switches: SmallVec<[(usize, u8); 2]>,
+ /// Version in effect at index 0 of the current batch, captured when the
+ /// FIRST switch is recorded — by then `protocol_version` already holds the
+ /// new one.
+ pub proto_batch_start: u8,
pub acl_log: AclLog,
/// Cached per-connection: true when the current user has no ACL
@@ -371,6 +390,8 @@ impl ConnectionState {
client_name,
asking: false,
readonly: false,
+ proto_switches: SmallVec::new(),
+ proto_batch_start: 2,
acl_log: AclLog::new(acl_max_len),
subscription_count: 0,
subscriber_id: 0,
diff --git a/src/server/conn/handler_monoio/dispatch.rs b/src/server/conn/handler_monoio/dispatch.rs
index 268d399d..5cf51baf 100644
--- a/src/server/conn/handler_monoio/dispatch.rs
+++ b/src/server/conn/handler_monoio/dispatch.rs
@@ -100,9 +100,15 @@ pub(super) fn check_auth_gate(
),
);
if !matches!(&response, Frame::Error(_)) {
+ // MUST come before `conn.protocol_version` moves: the helper
+ // reads it to learn what this batch STARTED in. Recorded at
+ // `responses.len()`, the index this reply will occupy, so the
+ // switch covers HELLO's own answer; replies already queued were
+ // produced under the OLD protocol and keep it. See
+ // `shared::encode_response_batch`.
+ crate::server::conn::shared::note_protocol_switch(conn, responses.len(), new_proto);
conn.protocol_version = new_proto;
- // Keep the wire codec in lockstep: the HELLO reply itself must
- // already be serialized in the negotiated protocol (RESP3 map).
+ // Keep the wire codec in lockstep for single-frame encodes.
codec.set_protocol_version(new_proto);
}
if let Some(name) = new_name {
@@ -408,9 +414,14 @@ pub(super) fn try_handle_hello(
),
);
if !matches!(&response, Frame::Error(_)) {
+ // MUST come before `conn.protocol_version` moves: the helper reads it
+ // to learn what this batch STARTED in. Recorded at `responses.len()`,
+ // the index this reply will occupy, so the switch covers HELLO's own
+ // answer; replies already queued were produced under the OLD protocol
+ // and keep it. See `shared::encode_response_batch`.
+ crate::server::conn::shared::note_protocol_switch(conn, responses.len(), new_proto);
conn.protocol_version = new_proto;
- // Keep the wire codec in lockstep: the HELLO reply itself must
- // already be serialized in the negotiated protocol (RESP3 map).
+ // Keep the wire codec in lockstep for single-frame encodes.
codec.set_protocol_version(new_proto);
}
if let Some(name) = new_name {
diff --git a/src/server/conn/handler_monoio/mod.rs b/src/server/conn/handler_monoio/mod.rs
index 80880dff..2c8d1b1c 100644
--- a/src/server/conn/handler_monoio/mod.rs
+++ b/src/server/conn/handler_monoio/mod.rs
@@ -1919,9 +1919,11 @@ pub(crate) async fn handle_connection_sharded_monoio<
&mut responses,
)
.await;
- for resp in &responses {
- codec.encode_frame(resp, &mut write_buf);
- }
+ crate::server::conn::shared::encode_response_batch(
+ &mut conn,
+ &responses,
+ &mut write_buf,
+ );
if !write_buf.is_empty() {
let data = write_buf.split().freeze();
if !write_all_bounded!(
@@ -3536,9 +3538,9 @@ pub(crate) async fn handle_connection_sharded_monoio<
}
// Serialize all responses into write_buf, then do ONE write_all syscall.
- for response in &responses {
- codec.encode_frame(response, &mut write_buf);
- }
+ // `encode_batch`, not a bare loop: a pipelined HELLO changes the protocol
+ // partway through and the replies before it must keep the old encoding.
+ crate::server::conn::shared::encode_response_batch(&mut conn, &responses, &mut write_buf);
// Write all responses in one batch using ownership I/O
if !write_buf.is_empty() {
diff --git a/src/server/conn/handler_sharded/mod.rs b/src/server/conn/handler_sharded/mod.rs
index f462a478..7e7b9dfe 100644
--- a/src/server/conn/handler_sharded/mod.rs
+++ b/src/server/conn/handler_sharded/mod.rs
@@ -675,6 +675,16 @@ pub(crate) async fn handle_connection_sharded_inner<
),
);
if !matches!(&response, Frame::Error(_)) {
+ // Recorded BEFORE the push, so the index is
+ // the one this reply will occupy and the
+ // switch covers it. Replies already queued
+ // were produced under the OLD protocol and
+ // keep it — see `encode_response_batch`.
+ crate::server::conn::shared::note_protocol_switch(
+ &mut conn,
+ responses.len(),
+ new_proto,
+ );
conn.protocol_version = new_proto;
}
if let Some(name) = new_name {
@@ -808,7 +818,10 @@ pub(crate) async fn handle_connection_sharded_inner<
ctx.cluster_state.is_some(),
),
);
- if !matches!(&response, Frame::Error(_)) { conn.protocol_version = new_proto; }
+ if !matches!(&response, Frame::Error(_)) {
+ crate::server::conn::shared::note_protocol_switch(&mut conn, responses.len(), new_proto);
+ conn.protocol_version = new_proto;
+ }
if let Some(name) = new_name { conn.client_name = Some(name); }
if let Some(ref uname) = opt_user {
conn.adopt_user(uname.clone(), &ctx.acl_table);
@@ -1415,13 +1428,14 @@ pub(crate) async fn handle_connection_sharded_inner<
)
.await;
write_buf.clear();
- for response in responses.iter() {
- if conn.protocol_version >= 3 {
- crate::protocol::serialize_resp3(response, &mut write_buf);
- } else {
- crate::protocol::serialize(response, &mut write_buf);
- }
- }
+ // Not a per-response version test: a pipelined HELLO
+ // moves the protocol partway through the batch, and the
+ // replies produced before it must keep the old encoding.
+ crate::server::conn::shared::encode_response_batch(
+ &mut conn,
+ &responses,
+ &mut write_buf,
+ );
if !write_all_bounded!(stream, &write_buf, write_timeout, out_cap_normal, client_live, client_id) { arena.reset(); return (HandlerResult::Done, None); }
// c10k A1: `read_buf` doubles as the carry buffer — it
// holds only the unparsed tail of this batch here, so
@@ -2711,13 +2725,14 @@ pub(crate) async fn handle_connection_sharded_inner<
}
write_buf.clear();
- for response in &responses {
- if conn.protocol_version >= 3 {
- crate::protocol::serialize_resp3(response, &mut write_buf);
- } else {
- crate::protocol::serialize(response, &mut write_buf);
- }
- }
+ // Not a per-response version test: a pipelined HELLO moves the
+ // protocol partway through the batch, and the replies produced
+ // before it must keep the old encoding.
+ crate::server::conn::shared::encode_response_batch(
+ &mut conn,
+ &responses,
+ &mut write_buf,
+ );
if !write_all_bounded!(stream, &write_buf, write_timeout, out_cap_normal, client_live, client_id) {
return (HandlerResult::Done, None);
}
diff --git a/src/server/conn/handler_single.rs b/src/server/conn/handler_single.rs
index 4868bda5..26e41a4d 100644
--- a/src/server/conn/handler_single.rs
+++ b/src/server/conn/handler_single.rs
@@ -530,6 +530,16 @@ pub async fn handle_connection(
// Phase 1: Handle connection-level intercepts, collect dispatchable frames
// Phase 2: Acquire ONE write lock, execute ALL dispatchable frames
let mut responses: Vec = Vec::with_capacity(batch.len());
+ // This handler flushes through `Framed::send`, which encodes each frame
+ // with the codec's version at send time, so it does NOT consume the
+ // per-reply switch record the two SHIPPED handlers use — and it shares
+ // `try_handle_reset`, which writes one. Dropped at the batch boundary so
+ // the record cannot accumulate across batches on a connection that
+ // RESETs repeatedly. This path is a library/embedding entry point
+ // (`server::run`); `main.rs` drives `run_sharded` at both call sites, so
+ // the binary never reaches it. Its own retro-encode behaviour is
+ // unchanged and recorded as a spec delta.
+ conn.proto_switches.clear();
// Each entry carries (resp_idx, db, bytes) so the Always-policy flush
// path can patch responses[resp_idx] with WRITEFAIL when fsync fails,
// before any response is sent to the client (H1 fix — FIX-W1-1).
diff --git a/src/server/conn/shared.rs b/src/server/conn/shared.rs
index a3e69ffd..eb373293 100644
--- a/src/server/conn/shared.rs
+++ b/src/server/conn/shared.rs
@@ -6,7 +6,6 @@ use std::sync::Arc;
use parking_lot::RwLock;
use bytes::Bytes;
-#[cfg(feature = "runtime-tokio")]
use bytes::BytesMut;
use crate::command::config as config_cmd;
@@ -27,6 +26,107 @@ use super::util::extract_command;
#[cfg(feature = "runtime-tokio")]
pub(crate) type SharedDatabases = Arc>>;
+/// Record that the reply at `at` — and every reply after it in this batch — is
+/// produced under `version`.
+///
+/// Call from the `HELLO` handler with `responses.len()` BEFORE pushing the
+/// HELLO reply, so the switch covers that reply too: Redis renders `HELLO`'s own
+/// answer in the protocol it has just negotiated.
+///
+/// **Call this BEFORE assigning `conn.protocol_version`.** The first call in a
+/// batch reads that field to learn what the batch STARTED in; calling it after
+/// the assignment records the new version as the batch start and silently
+/// restores the very bug this exists to fix. Only an end-to-end test can catch
+/// that mistake — `tests/batch_protocol_version.rs::bpv1` and `::bpv3` are the
+/// pins, and they DID catch it during development.
+pub(crate) fn note_protocol_switch(
+ conn: &mut super::core::ConnectionState,
+ at: usize,
+ version: u8,
+) {
+ if conn.proto_switches.is_empty() {
+ conn.proto_batch_start = conn.protocol_version;
+ }
+ conn.proto_switches.push((at, version));
+}
+
+/// Walks a batch index-by-index, yielding the protocol version in effect at
+/// each one.
+///
+/// Split out from `encode_response_batch` so the version arithmetic — the part
+/// that is easy to get subtly wrong and expensive to observe on the wire — can
+/// be tested directly. Indices must be visited in ascending order; the cursor
+/// never rewinds, which keeps the walk O(batch) rather than O(batch x switches).
+struct ProtoWalk<'a> {
+ switches: &'a [(usize, u8)],
+ next: usize,
+ version: u8,
+}
+
+impl<'a> ProtoWalk<'a> {
+ fn new(start: u8, switches: &'a [(usize, u8)]) -> Self {
+ Self {
+ switches,
+ next: 0,
+ version: start,
+ }
+ }
+
+ /// Version for reply `idx`. A switch recorded AT `idx` applies to it —
+ /// `HELLO`'s own reply is rendered in the protocol it just negotiated.
+ fn version_at(&mut self, idx: usize) -> u8 {
+ while let Some(&(at, to)) = self.switches.get(self.next)
+ && at <= idx
+ {
+ self.version = to;
+ self.next += 1;
+ }
+ self.version
+ }
+}
+
+/// Serialize a whole response batch, honouring any protocol switch recorded
+/// inside it, then clear the switch list ready for the next batch.
+///
+/// A pipelined `HELLO` moves `conn.protocol_version` the instant it is handled,
+/// but the batch is not serialized until every command in it has run. Encoding
+/// the batch under one final version therefore RETRO-encodes the replies
+/// produced before the switch — measured against redis-server 8.6.1, a
+/// `CONFIG GET` answered under RESP3 then followed by `HELLO 2` in the same
+/// pipeline must still go out as `%1`, not `*2`.
+///
+/// With no switch recorded — every batch that contains no `HELLO`, which is
+/// essentially all of them — this is exactly the single-version loop it
+/// replaces, with one branch and no allocation.
+pub(crate) fn encode_response_batch(
+ conn: &mut super::core::ConnectionState,
+ responses: &[Frame],
+ buf: &mut BytesMut,
+) {
+ if conn.proto_switches.is_empty() {
+ if conn.protocol_version >= 3 {
+ for item in responses {
+ crate::protocol::serialize_resp3(item, buf);
+ }
+ } else {
+ for item in responses {
+ crate::protocol::serialize(item, buf);
+ }
+ }
+ return;
+ }
+
+ let mut walk = ProtoWalk::new(conn.proto_batch_start, &conn.proto_switches);
+ for (idx, item) in responses.iter().enumerate() {
+ if walk.version_at(idx) >= 3 {
+ crate::protocol::serialize_resp3(item, buf);
+ } else {
+ crate::protocol::serialize(item, buf);
+ }
+ }
+ conn.proto_switches.clear();
+}
+
/// Resolve FT.SEARCH `as_of_lsn` with the canonical precedence (TEMP-04, ACID-09):
///
/// 1. Explicit `AS_OF ` clause -> `TemporalRegistry::lsn_at(wall_ms)`.
@@ -1244,6 +1344,15 @@ pub(crate) fn try_handle_reset(
// Identity + protocol, from the one definition of "default".
let (proto, db, authed, user, name) =
crate::server::conn::util::restore_migrated_state(None, requirepass);
+ // RESET is the SECOND protocol switch in the command set, and it moves the
+ // protocol the same way a pipelined `HELLO 2` does. Recorded before the
+ // assignment, at the index `+RESET` will occupy, so replies produced
+ // earlier in this batch keep the protocol they were produced under. A fix
+ // that covered only HELLO left `HELLO 3` + `RESET` in one write still
+ // retro-downgrading — see `note_protocol_switch`.
+ if proto != conn.protocol_version {
+ note_protocol_switch(conn, responses.len(), proto);
+ }
conn.protocol_version = proto;
conn.selected_db = db;
conn.authenticated = authed;
@@ -1841,3 +1950,45 @@ mod watch_locality_tests {
);
}
}
+
+#[cfg(test)]
+mod proto_walk_tests {
+ use super::ProtoWalk;
+
+ fn seq(start: u8, switches: &[(usize, u8)], n: usize) -> Vec {
+ let mut w = ProtoWalk::new(start, switches);
+ (0..n).map(|i| w.version_at(i)).collect()
+ }
+
+ /// The defect this whole mechanism exists for: a reply produced under RESP3
+ /// keeps RESP3 even though a later HELLO 2 downgraded the connection.
+ #[test]
+ fn a_switch_does_not_reach_backwards() {
+ assert_eq!(seq(3, &[(1, 2)], 3), vec![3, 2, 2]);
+ }
+
+ /// HELLO's own reply is rendered in the protocol it just negotiated, so the
+ /// switch applies AT its index, not after it.
+ #[test]
+ fn a_switch_applies_at_its_own_index_not_the_next_one() {
+ assert_eq!(seq(2, &[(0, 3)], 2), vec![3, 3]);
+ }
+
+ /// Two HELLOs in one batch: each takes effect from its own index.
+ #[test]
+ fn every_switch_in_a_batch_is_honoured_in_order() {
+ assert_eq!(seq(3, &[(1, 2), (3, 3)], 5), vec![3, 2, 2, 3, 3]);
+ }
+
+ /// The overwhelmingly common case — no HELLO in the batch.
+ #[test]
+ fn no_switches_means_one_version_throughout() {
+ assert_eq!(seq(3, &[], 4), vec![3, 3, 3, 3]);
+ }
+
+ /// A switch past the end of the batch cannot affect it, and must not panic.
+ #[test]
+ fn a_switch_beyond_the_last_reply_is_inert() {
+ assert_eq!(seq(2, &[(9, 3)], 3), vec![2, 2, 2]);
+ }
+}
diff --git a/tests/batch_protocol_version.rs b/tests/batch_protocol_version.rs
new file mode 100644
index 00000000..325a7f86
--- /dev/null
+++ b/tests/batch_protocol_version.rs
@@ -0,0 +1,419 @@
+//! ADD task `batch-protocol-version-fidelity` — failing-first suite.
+//!
+//! One rule, stated once: **every reply is encoded in the protocol that was in
+//! effect when that reply was produced.** A `HELLO` in the middle of a pipeline
+//! changes the protocol for the replies that come AFTER it (and for its own
+//! reply), never for the ones already produced.
+//!
+//! Moon serializes a whole batch at flush time under a single
+//! `codec.protocol_version`, while `codec.set_protocol_version` fires
+//! synchronously at the HELLO site mid-batch. The post-HELLO version therefore
+//! retro-encodes replies produced before it.
+//!
+//! The UPGRADE direction (`HELLO 3` mid-batch) happens to look right today —
+//! not because the batch is encoded correctly, but because
+//! `apply_resp3_conversion` already flattened the earlier reply to an `Array`
+//! at dispatch time under RESP2, so re-encoding it as RESP3 still emits `*`.
+//! The DOWNGRADE direction has no such accident and is visibly wrong. Both
+//! directions are asserted, so a fix cannot trade one for the other.
+//!
+//! Oracle: redis-server 8.6.1, measured 2026-08-14 over a raw socket.
+//!
+//! pipelined CONFIG GET maxmemory / HELLO 2 / CONFIG GET maxmemory
+//! on a RESP3 connection:
+//! redis -> %1 (produced under RESP3) *14 (HELLO's own reply) *2
+//! moon -> *2 *14 *2
+//! ^ wrong: retro-downgraded
+//!
+//! Reproduced on monoio and tokio, shards 1 and 4 — all three dispatch paths.
+//!
+//! Run alone with: cargo test --test batch_protocol_version
+
+mod common;
+
+use std::io::{Read, Write};
+use std::net::TcpStream;
+use std::process::{Child, Command};
+use std::time::{Duration, Instant};
+
+// ---------------------------------------------------------------------------
+// Harness
+// ---------------------------------------------------------------------------
+
+fn spawn_moon(dir: &std::path::Path, shards: u32) -> (Child, u16) {
+ common::spawn_listening(|port| {
+ Command::new(common::find_moon_binary())
+ .args([
+ "--port",
+ &port.to_string(),
+ "--dir",
+ &dir.to_string_lossy(),
+ "--shards",
+ &shards.to_string(),
+ "--appendonly",
+ "no",
+ // The shared /Volumes checkout hovers near the 5% diskfull
+ // guard; a tripped guard would fail this suite for an unrelated
+ // reason.
+ "--disk-free-min-pct",
+ "0",
+ ])
+ .stdout(std::fs::File::create(dir.join("moon.stdout.log")).expect("stdout log"))
+ .stderr(std::fs::File::create(dir.join("moon.stderr.log")).expect("stderr log"))
+ .spawn()
+ .expect("spawn moon")
+ })
+}
+
+struct ServerGuard(Child);
+impl Drop for ServerGuard {
+ fn drop(&mut self) {
+ common::sigkill(&mut self.0);
+ }
+}
+
+fn connect_ready(port: u16) -> TcpStream {
+ let deadline = Instant::now() + Duration::from_secs(30);
+ loop {
+ if let Ok(s) = TcpStream::connect(format!("127.0.0.1:{port}")) {
+ s.set_read_timeout(Some(Duration::from_secs(10))).ok();
+ s.set_write_timeout(Some(Duration::from_secs(10))).ok();
+ let mut s = s;
+ if s.write_all(b"PING\r\n").is_ok() {
+ let mut buf = [0u8; 64];
+ if let Ok(n) = s.read(&mut buf)
+ && n > 0
+ && buf[..n].windows(4).any(|w| w == b"PONG")
+ {
+ return s;
+ }
+ }
+ }
+ assert!(
+ Instant::now() < deadline,
+ "server on {port} never answered PING"
+ );
+ std::thread::sleep(Duration::from_millis(50));
+ }
+}
+
+/// A reply reduced to what this suite is about: its RESP type byte.
+///
+/// Values are deliberately discarded. Two protocols may legitimately render
+/// the same value differently; what must be right is which protocol was used.
+struct Conn {
+ s: TcpStream,
+ buf: Vec,
+ pos: usize,
+}
+
+impl Conn {
+ fn new(port: u16, proto: u8) -> Self {
+ let mut c = Conn {
+ s: connect_ready(port),
+ buf: Vec::with_capacity(64 * 1024),
+ pos: 0,
+ };
+ if proto == 3 {
+ c.write(&[&["HELLO", "3"]]);
+ let tag = c.skip_frame();
+ assert_eq!(
+ tag, '%',
+ "HELLO 3 must be answered with a RESP3 map or this suite proves nothing"
+ );
+ }
+ c
+ }
+
+ /// Write every command in ONE `write_all`, so the server sees a single
+ /// pipelined batch rather than a sequence of round trips. That is the whole
+ /// point: a batch is what gets encoded under one protocol version.
+ fn write(&mut self, batch: &[&[&str]]) {
+ let mut req = Vec::with_capacity(256);
+ for parts in batch {
+ req.extend_from_slice(format!("*{}\r\n", parts.len()).as_bytes());
+ for p in *parts {
+ req.extend_from_slice(format!("${}\r\n{p}\r\n", p.len()).as_bytes());
+ }
+ }
+ self.s.write_all(&req).expect("write batch");
+ }
+
+ fn fill(&mut self) {
+ let mut chunk = [0u8; 16 * 1024];
+ let n = self.s.read(&mut chunk).expect("read");
+ assert!(n > 0, "connection closed mid-frame");
+ self.buf.extend_from_slice(&chunk[..n]);
+ }
+
+ fn line(&mut self) -> String {
+ loop {
+ if let Some(rel) = self.buf[self.pos..].windows(2).position(|w| w == b"\r\n") {
+ let start = self.pos;
+ let end = start + rel;
+ let out = String::from_utf8_lossy(&self.buf[start..end]).into_owned();
+ self.pos = end + 2;
+ return out;
+ }
+ self.fill();
+ }
+ }
+
+ fn exact(&mut self, n: usize) {
+ while self.buf.len() - self.pos < n + 2 {
+ self.fill();
+ }
+ self.pos += n + 2;
+ }
+
+ /// Consume exactly one reply and return its top-level type byte.
+ fn skip_frame(&mut self) -> char {
+ let line = self.line();
+ let tag = line.chars().next().expect("empty frame");
+ let rest = &line[1..];
+ match tag {
+ '+' | '-' | ':' | ',' | '#' | '_' | '(' => {}
+ '$' | '=' => {
+ let n: i64 = rest.parse().unwrap_or(-1);
+ if n >= 0 {
+ self.exact(n as usize);
+ }
+ }
+ '*' | '~' | '>' => {
+ let n: i64 = rest.parse().unwrap_or(-1);
+ for _ in 0..n.max(0) {
+ self.skip_frame();
+ }
+ }
+ '%' => {
+ let n: i64 = rest.parse().unwrap_or(-1);
+ for _ in 0..n.max(0) * 2 {
+ self.skip_frame();
+ }
+ }
+ other => panic!("unknown RESP type byte {other:?} in {line:?}"),
+ }
+ tag
+ }
+
+ /// Type byte of each of the next `n` replies, in order.
+ fn tags(&mut self, n: usize) -> String {
+ (0..n).map(|_| self.skip_frame()).collect()
+ }
+}
+
+/// Run `body` against a server on 1 shard and on 4 shards.
+///
+/// Not decoration: `--shards 1` and `--shards 4` reach different dispatch
+/// handlers, and this repo's recurring defect class is a behaviour that exists
+/// on some dispatch paths and not others.
+fn on_each_shard_count(body: impl Fn(u16)) {
+ for shards in [1u32, 4] {
+ let dir = tempfile::tempdir().expect("tempdir");
+ let (child, port) = spawn_moon(dir.path(), shards);
+ let _guard = ServerGuard(child);
+ body(port);
+ }
+}
+
+// ---------------------------------------------------------------------------
+// bpv1-bpv4 — protocol in effect at production time
+// ---------------------------------------------------------------------------
+
+/// RED on main. The load-bearing test: a reply produced under RESP3 must stay
+/// RESP3 even though a later `HELLO 2` in the same batch downgraded the
+/// connection.
+#[test]
+fn bpv1_a_reply_produced_before_hello_2_keeps_its_resp3_encoding() {
+ on_each_shard_count(|port| {
+ let mut c = Conn::new(port, 3);
+ c.write(&[
+ &["CONFIG", "GET", "maxmemory"],
+ &["HELLO", "2"],
+ &["CONFIG", "GET", "maxmemory"],
+ ]);
+ assert_eq!(
+ c.tags(3),
+ "%**",
+ "reply 1 was produced while RESP3 was in effect and must be a Map; \
+ HELLO 2's own reply and everything after it are RESP2 arrays"
+ );
+ });
+}
+
+/// The mirror direction. Expected GREEN today — a pin, so the fix for `bpv1`
+/// cannot be a blanket "use the batch-start version" that breaks this.
+#[test]
+fn bpv2_a_reply_produced_before_hello_3_keeps_its_resp2_encoding() {
+ on_each_shard_count(|port| {
+ let mut c = Conn::new(port, 2);
+ c.write(&[
+ &["CONFIG", "GET", "maxmemory"],
+ &["HELLO", "3"],
+ &["CONFIG", "GET", "maxmemory"],
+ ]);
+ assert_eq!(
+ c.tags(3),
+ "*%%",
+ "reply 1 was produced under RESP2 and must stay an Array; HELLO 3's \
+ own reply and everything after it are RESP3 maps"
+ );
+ });
+}
+
+/// RED on main. Two switches in one batch — proves the fix tracks a SEQUENCE of
+/// switch points, not a single "did a HELLO happen" flag.
+#[test]
+fn bpv3_two_hellos_in_one_batch_each_take_effect_from_their_own_index() {
+ on_each_shard_count(|port| {
+ let mut c = Conn::new(port, 3);
+ c.write(&[
+ &["CONFIG", "GET", "maxmemory"], // % produced under RESP3
+ &["HELLO", "2"], // * switch -> RESP2, own reply RESP2
+ &["CONFIG", "GET", "maxmemory"], // * RESP2
+ &["HELLO", "3"], // % switch -> RESP3, own reply RESP3
+ &["CONFIG", "GET", "maxmemory"], // % RESP3
+ ]);
+ assert_eq!(c.tags(5), "%**%%", "each HELLO applies from its own index");
+ });
+}
+
+/// Expected GREEN today. Pins the hot path: a batch with no HELLO in it must be
+/// encoded exactly as before, since that is every real pipeline.
+#[test]
+fn bpv4_a_batch_without_hello_is_encoded_entirely_in_one_protocol() {
+ on_each_shard_count(|port| {
+ let mut c = Conn::new(port, 3);
+ c.write(&[
+ &["CONFIG", "GET", "maxmemory"],
+ &["SET", "bpv4", "v"],
+ &["CONFIG", "GET", "maxmemory"],
+ ]);
+ assert_eq!(c.tags(3), "%+%", "no switch point, no change in encoding");
+
+ let mut c2 = Conn::new(port, 2);
+ c2.write(&[
+ &["CONFIG", "GET", "maxmemory"],
+ &["SET", "bpv4b", "v"],
+ &["CONFIG", "GET", "maxmemory"],
+ ]);
+ assert_eq!(c2.tags(3), "*+*", "same, under RESP2");
+ });
+}
+
+/// RED on main. `HELLO` is not the only command that moves the protocol —
+/// `RESET` is contracted to return the connection to its default state, which
+/// includes RESP2. §0 measured `HELLO 3` + `RESET` in one write producing `*14`
+/// for the HELLO reply, exactly like the `HELLO 2` case.
+///
+/// Kept as its own test rather than folded into bpv1 because it fails through a
+/// different code path: `shared::try_handle_reset`, not the two HELLO sites. A
+/// fix that covers only HELLO leaves this red — which is what it did.
+#[test]
+fn bpv7_reset_is_a_protocol_switch_and_does_not_reach_backwards() {
+ on_each_shard_count(|port| {
+ let mut c = Conn::new(port, 3);
+ c.write(&[
+ &["CONFIG", "GET", "maxmemory"], // % produced under RESP3
+ &["RESET"], // + switch -> RESP2 (reply is +RESET either way)
+ &["CONFIG", "GET", "maxmemory"], // * RESP2
+ ]);
+ assert_eq!(
+ c.tags(3),
+ "%+*",
+ "RESET reverts to RESP2 from its own index onward; the reply produced \
+ before it stays a RESP3 map"
+ );
+ });
+}
+
+// ---------------------------------------------------------------------------
+// bpv5-bpv6 — CONFIG GET accepts more than one parameter
+// ---------------------------------------------------------------------------
+//
+// Found while measuring the oracle for the batch tests. Moon reads only
+// `args[0]` and silently drops the rest, so `CONFIG GET maxmemory appendonly`
+// answers with maxmemory alone. Glob patterns work; multiple parameters do not.
+// `redis-py`'s `config_get(*params)` and every monitoring agent that reads two
+// settings in one call hit this.
+//
+// Measured on redis-server 8.6.1:
+// CONFIG GET maxmemory appendonly -> both, in the server's own table order
+// CONFIG GET maxmemory 'maxmemory*'-> deduplicated; maxmemory appears ONCE
+// CONFIG GET nosuchparam maxmemory -> unknown patterns silently skipped
+// CONFIG GET nosuchparam -> empty array
+
+/// Read a CONFIG GET reply as the set of parameter names it returned.
+fn config_get_names(port: u16, args: &[&str]) -> Vec {
+ let mut parts: Vec<&str> = vec!["CONFIG", "GET"];
+ parts.extend_from_slice(args);
+ let mut c = Conn::new(port, 2); // RESP2: a flat array, easiest to read
+ c.write(&[&parts]);
+
+ let header = c.line();
+ assert_eq!(
+ &header[..1],
+ "*",
+ "RESP2 CONFIG GET is an Array: {header:?}"
+ );
+ let n: usize = header[1..].parse().expect("array length");
+ let mut names = Vec::new();
+ for i in 0..n {
+ let lead = c.line();
+ let len: i64 = lead[1..].parse().unwrap_or(-1);
+ let start = c.pos;
+ if len >= 0 {
+ c.exact(len as usize);
+ }
+ if i % 2 == 0 {
+ names.push(
+ String::from_utf8_lossy(&c.buf[start..start + len.max(0) as usize]).into_owned(),
+ );
+ }
+ }
+ names
+}
+
+/// RED on main — Moon returns only `maxmemory`.
+#[test]
+fn bpv5_config_get_honours_every_parameter_not_just_the_first() {
+ on_each_shard_count(|port| {
+ let mut names = config_get_names(port, &["maxmemory", "appendonly"]);
+ names.sort();
+ assert_eq!(
+ names,
+ vec!["appendonly".to_string(), "maxmemory".to_string()],
+ "every supplied parameter must be answered, not just args[0]"
+ );
+
+ assert!(
+ config_get_names(port, &["nosuchparam", "maxmemory"]) == vec!["maxmemory".to_string()],
+ "an unknown parameter is skipped, not an error, and must not \
+ suppress the known ones beside it"
+ );
+ assert!(
+ config_get_names(port, &["nosuchparam"]).is_empty(),
+ "all-unknown answers an empty array"
+ );
+ });
+}
+
+/// RED on main. Overlapping patterns must not double-report a parameter.
+#[test]
+fn bpv6_config_get_deduplicates_overlapping_patterns() {
+ on_each_shard_count(|port| {
+ let names = config_get_names(port, &["maxmemory", "maxmemory*"]);
+ let mut seen = names.clone();
+ seen.sort();
+ seen.dedup();
+ assert_eq!(
+ seen.len(),
+ names.len(),
+ "a parameter matched by two patterns must appear once: {names:?}"
+ );
+ assert!(
+ names.iter().any(|n| n == "maxmemory"),
+ "the exact-match parameter is still present: {names:?}"
+ );
+ });
+}