fix(protocol): name protocol faults on the wire, and make MULTI atomic at queue time

.add/state.json

@@ -1,7 +1,7 @@
 {
   "project": "moon",
   "stage": "production",
-  "active_task": "client-identity-introspection",
+  "active_task": "protocol-error-lifetime",
   "active_milestone": "v0-9-client-compat",
   "tasks": {
     "hotpath-lock-quickwins": {
@@ -332,14 +332,14 @@
     },
     "protocol-error-lifetime": {
       "title": "Protocol errors reply and close cleanly, never stall or eat the valid prefix",
-      "phase": "ground",
+      "phase": "tests",
       "gate": "none",
       "milestone": "v0-9-client-compat",
       "depends_on": [
         "client-compat-harness"
       ],
       "created": "2026-08-09T16:42:35+00:00",
-      "updated": "2026-08-09T16:42:35+00:00"
+      "updated": "2026-08-12T04:55:05+00:00"
     },
     "multi-exec-queue-semantics": {
       "title": "MULTI queues every command, and EXEC answers correctly on every shard count",
@@ -476,7 +476,7 @@
     }
   },
   "created": "2026-06-11T03:18:21+00:00",
-  "updated": "2026-08-11T18:20:42+00:00",
+  "updated": "2026-08-12T04:55:06+00:00",
   "setup": {
     "locked": true,
     "locked_at": "2026-06-11T03:28:00+00:00",

CHANGELOG.md

@@ -6,6 +6,30 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+- **A malformed frame no longer closes the connection silently, and no longer eats the valid
+  commands that arrived with it.** `Err(_) => break` in the read loops discarded two things: the
+  parse error's reason, so a client got a bare FIN and could not tell a bad encoder from a dropped
+  network, and the already-parsed batch — so `PING\r\n*-9\r\n` in a single write answered
+  *nothing at all*. All three handlers now execute and flush the valid prefix, then send
+  `-ERR Protocol error: <reason>` using redis-server 8.6.1's verbatim wording, then close. Also
+  measured and fixed alongside it: `*-9` (any negative multibulk count) killed the connection where
+  Redis ignores it; an oversized inline request closed mute even though Moon already *built* the
+  correct "too big inline request" message; and inline quoting did not exist at all, so
+  `SET k "a b"` became three arguments containing literal quote bytes and `GET "unclosed` was
+  silently accepted as a key — the inline parser is now a port of Redis's `sdssplitargs`.
+- **`MULTI` is atomic with respect to queue-time faults.** Moon had no queue-time validation, so
+  `EXEC` ran whichever half of a transaction happened to parse: `MULTI / NOSUCHCMD / SET k v / EXEC`
+  left `k` **set** where Redis discards everything — data corruption, not a compatibility nit. A
+  command that could never run (unknown name, impossible arity, `SUBSCRIBE`, `WATCH`) is now refused
+  as it is queued and poisons the transaction, and `EXEC` answers
+  `-EXECABORT Transaction discarded because of previous errors.` Fixing this exposed a wider defect:
+  Moon decided "am I in a transaction?" hundreds of lines *below* the `INFO` / `CLIENT` / `WS` /
+  `MQ` / `PUBLISH` / `SUBSCRIBE` intercepts, so every one of those executed for real inside a
+  transaction — `SUBSCRIBE ch` put the connection into subscriber mode mid-`MULTI`, and
+  `INFO server` returned a 3 KB dump where Redis returns `+QUEUED`. The queue decision now sits
+  directly below each handler's ACL gate, which fixes the whole class at once.
+
 ### Added
 - **`ROLE`, `RESET`, and a real `COMMAND` introspection surface.** `COMMAND` and `COMMAND COUNT`
   each returned the OTHER'S RESP TYPE — bare `COMMAND` replied `:0` (an Integer where an Array

scripts/client-compat/manifest.yaml

@@ -93,27 +93,23 @@ entries:
         transaction queueing, distinct from the inline-GET path.
         Owner: multi-exec-queue-semantics (re-owned 2026-08-10: a
         transaction-QUEUEING defect, not a reply-type one).
-
+        STILL REPRODUCES 2026-08-12 after the MULTI queue gate landed, which
+        DID fix the same class for INFO and SUBSCRIBE. CONFIG sits below the
+        gate in every handler, so the reason is not intercept order and is not
+        yet understood — deliberately not guessed at here.
+
+  # FIXED 2026-08-12 by multi-exec-queue-semantics. The waivers are retired
+  # rather than deleted: these two are now live parity assertions, so the
+  # EXECABORT behaviour cannot silently regress. Moon used to queue the bad
+  # command and surface its -ERR from inside EXEC, leaving the transaction
+  # PARTIALLY APPLIED — `MULTI / NOSUCHCMD / SET k v / EXEC` left k set.
   - name: multi_aborts_on_unknown_command
     command: "THISCOMMANDDOESNOTEXIST"
     policy: exact
-    expect_diff:
-      reason: >-
-        Redis fails the queue-time error and then answers EXEC with
-        -EXECABORT "Transaction discarded because of previous errors."; Moon
-        queues the bad command and surfaces the -ERR from inside EXEC instead.
-        A client that relies on EXECABORT to detect a poisoned transaction sees
-        a partially-applied one. Owner: multi-exec-queue-semantics (re-owned
-        2026-08-10: a transaction-QUEUEING defect, not a reply-type one).
 
   - name: multi_aborts_on_wrong_arity
     command: "GET"
     policy: exact
-    expect_diff:
-      reason: >-
-        Same EXECABORT gap as multi_aborts_on_unknown_command, reached through
-        an arity error rather than an unknown name.
-        Owner: multi-exec-queue-semantics.
 
   # ── RESP3 reply-type fidelity (review findings, wire-confirmed) ───────
 
@@ -209,6 +205,8 @@ entries:
         TYPE is correct in every context where the command actually runs
         (standalone and pipeline both answer %0); this is the transaction
         QUEUEING gap, not a reply-shape one.
+        STILL REPRODUCES 2026-08-12 after the MULTI queue gate landed — see
+        multi_queues_config_get for the same unexplained residual.
         Owner: multi-exec-queue-semantics.
 
   - name: empty_smembers_is_still_a_set

src/protocol/frame.rs

@@ -217,6 +217,77 @@ impl PartialEq for Frame {
     }
 }
 
+/// Which protocol fault occurred, in the vocabulary Redis puts on the wire.
+///
+/// `ParseError::Invalid` already carries a detailed `message` — it is what a
+/// fuzz triage or a log reader wants ("invalid bulk string length: -5"
+/// localises a bug; "invalid bulk length" does not). But that detail is
+/// Moon's own wording, and a driver author reading it cannot match it against
+/// the Redis error they wrote their reconnect logic around.
+///
+/// So the fault carries BOTH: this enum for the wire, the message for us.
+/// Redis's set is small and fixed (`networking.c`), which is why this is a
+/// closed enum of `&'static str` rather than a formatted string — nothing
+/// here allocates, and it is only ever reached on a connection that is
+/// already terminating.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum ProtoFault {
+    /// A bulk header's length was absent, non-numeric, negative (other than
+    /// the `-1` null), or past the configured maximum.
+    BulkLen,
+    /// An array/set/map/push header's element count was malformed or too deep.
+    MultibulkLen,
+    /// An array element did not begin with `$`. Carries the offending byte.
+    ExpectedDollar(u8),
+    /// An inline request exceeded the inline cap.
+    InlineTooBig,
+    /// An inline request ended with a quote still open.
+    UnbalancedQuotes,
+    /// The multibulk count string itself was implausibly long.
+    MbulkCountTooBig,
+    /// A type byte Moon parses but a Redis client should never send inbound.
+    /// Redis conflates this with `ExpectedDollar`, and so do we.
+    UnknownType(u8),
+}
+
+impl ProtoFault {
+    /// The reason text, verbatim from redis-server 8.6.1. The caller prefixes
+    /// `-ERR `. Returns `None` for the two byte-carrying variants, whose text
+    /// must be formatted — see [`ProtoFault::wire_text_owned`].
+    pub fn wire_text(&self) -> &'static str {
+        match self {
+            ProtoFault::BulkLen => "Protocol error: invalid bulk length",
+            ProtoFault::MultibulkLen => "Protocol error: invalid multibulk length",
+            ProtoFault::InlineTooBig => "Protocol error: too big inline request",
+            ProtoFault::UnbalancedQuotes => "Protocol error: unbalanced quotes in request",
+            ProtoFault::MbulkCountTooBig => "Protocol error: too big mbulk count string",
+            // Both byte-carrying variants render through `wire_text_owned`;
+            // this arm exists so a caller that only wants a static string
+            // still gets the right *shape* rather than a panic.
+            ProtoFault::ExpectedDollar(_) | ProtoFault::UnknownType(_) => {
+                "Protocol error: expected '$', got '?'"
+            }
+        }
+    }
+
+    /// The reason text with any offending byte substituted in.
+    ///
+    /// Allocates, deliberately: this runs once per doomed connection, never on
+    /// a serving path, and the alternative (a stack buffer threaded through
+    /// three handlers) buys nothing measurable.
+    pub fn wire_text_owned(&self) -> String {
+        match self {
+            ProtoFault::ExpectedDollar(b) | ProtoFault::UnknownType(b) => {
+                // Redis prints the raw byte. A non-printable one renders as
+                // whatever the terminal makes of it, which is Redis's
+                // behavior too — matching it matters more than prettiness.
+                format!("Protocol error: expected '$', got '{}'", *b as char)
+            }
+            other => other.wire_text().to_string(),
+        }
+    }
+}
+
 /// Errors that can occur when parsing RESP2 frames.
 #[derive(Debug, Error)]
 pub enum ParseError {
@@ -226,8 +297,15 @@ pub enum ParseError {
     Incomplete,
 
     /// The data violates the RESP2 protocol specification.
+    ///
+    /// `kind` is what goes on the wire; `message` is the detailed internal
+    /// reason kept for logs and fuzz triage. They are deliberately different.
     #[error("invalid frame at byte {offset}: {message}")]
-    Invalid { message: String, offset: usize },
+    Invalid {
+        kind: ProtoFault,
+        message: String,
+        offset: usize,
+    },
 
     /// An I/O error occurred while reading from the buffer.
     #[error("io error: {0}")]
@@ -302,9 +380,13 @@ mod tests {
     #[test]
     fn test_parse_error_invalid_display() {
         let err = ParseError::Invalid {
+            kind: ProtoFault::BulkLen,
             message: "bad".into(),
             offset: 5,
         };
+        // Display still renders the DETAILED internal message, not the wire
+        // text — the two are deliberately different, and this assertion is
+        // what pins that. `kind` travels alongside for the client's benefit.
         assert_eq!(format!("{}", err), "invalid frame at byte 5: bad");
     }