From 01cbb49bb2db48b2f24c0babf08398297e307ca8 Mon Sep 17 00:00:00 2001 From: Tin Dang Date: Sat, 15 Aug 2026 23:45:03 +0700 Subject: [PATCH] fix(registry): deregister five unreachable commands; gate reply shape across contexts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two v0-9-client-compat exit criteria were unmet. Both are closed here with measurement, not assertion. EC5 — "no command registered in metadata.rs answers unknown command". LATENCY, MODULE, DUMP, RESTORE and RECLAMATION were registered but dispatched nowhere, so COMMAND, COMMAND COUNT and ACL described a surface Moon cannot serve. A client that introspects before calling was told it could. The milestone's own wording is "implement or deregister"; these are deregistered. Three of the five were never top-level commands at all — RECLAMATION exists only as `DEBUG RECLAMATION`, DUMP/RESTORE only as `FUNCTION DUMP` / `FUNCTION RESTORE`. Those subcommands are untouched and their suites pass. COMMAND COUNT 267 -> 262. With these gone the registry sweep runs with NO waiver list at all: every entry is proven reachable on both feature legs. That list was 10 names this morning, and five of them were commands that had already shipped. EC6 — "same reply shape standalone, inside MULTI, and inside a pipeline". The harness has always supported `--contexts standalone,multi,pipeline`; CI only ever ran `--strict`, which covers standalone. So the rule was asserted by hand and never gated. Now its own step, rehearsed in the exact CI form first: PASS=201 FAIL=0 WAIVED=17 across all three contexts, exit 0. Evidence: lib, monoio (default): 4641 passed / 0 failed lib, tokio+jemalloc: 3807 passed / 0 failed wire_reachability_red: 3/3 on BOTH legs, zero waivers monitor_command_feed 27/27 · multi_exec_queue_semantics 12/12 · client_identity_introspection 12/12 · functions_fcall · vacuum_commands compat harness --strict --contexts standalone,multi,pipeline: exit 0 fmt + clippy clean on both feature legs author: Tin Dang --- .github/workflows/ci.yml | 12 ++++++++++++ CHANGELOG.md | 14 ++++++++++++++ src/command/metadata.rs | 23 +++++++++++++++-------- tests/wire_reachability_red.rs | 34 +++++++++++++++++++--------------- 4 files changed, 60 insertions(+), 23 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a3f70713..204b08bc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -405,6 +405,18 @@ jobs: env: MOON_BIN: ${{ env.CARGO_TARGET_DIR }}/release/moon timeout-minutes: 15 + # A command must not change shape by context. The harness could always + # prove this and CI never asked it to — it ran `--strict` alone, which + # covers standalone only, while the milestone exit criterion is about all + # three contexts. Measured before wiring: PASS=201 FAIL=0 WAIVED=17. + - name: Compatibility diff across contexts (standalone · MULTI · pipeline) + run: | + ./scripts/test-client-compat.sh --strict \ + --contexts standalone,multi,pipeline \ + --record tmp/client-compat-contexts.json + env: + MOON_BIN: ${{ env.CARGO_TARGET_DIR }}/release/moon + timeout-minutes: 20 - name: INFO field coverage run: ./scripts/test-client-compat.sh --filter __none__ --info-manifest --record tmp/client-compat-info.json env: diff --git a/CHANGELOG.md b/CHANGELOG.md index bb860545..70eef4fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -77,6 +77,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 dispatcher — so `COMMAND COUNT` was advertising verbs Moon could not run. ### Fixed +- **Five commands were advertised by `COMMAND`/ACL but dispatched nowhere; they are deregistered.** + `LATENCY`, `MODULE`, `DUMP`, `RESTORE` and `RECLAMATION` sat in `metadata.rs` while answering + `unknown command` on every dispatch path, so `COMMAND`, `COMMAND COUNT` (267 -> 262) and ACL all + described a surface Moon cannot serve — a client that introspects before calling was told it + could. Three were never top-level commands at all: `RECLAMATION` is reachable only as + `DEBUG RECLAMATION`, and `DUMP`/`RESTORE` only as `FUNCTION DUMP` / `FUNCTION RESTORE`, both of + which are unaffected. With these gone the registry sweep + (`cdg1_registry_sweep_no_unknowns`) runs with **no waiver list at all** — every entry in the + registry is now proven reachable on both feature legs, which is what the v0.9 exit criterion + asked for. +- **CI now proves reply shapes match across MULTI and pipeline, not just standalone.** The compat + harness has always supported `--contexts standalone,multi,pipeline` and CI only ever ran + `--strict`, so the "a command must not change shape by context" rule was asserted by hand and + never gated. Wired in as its own step (PASS=201 FAIL=0 across all three contexts). - **A test waiver went stale and hid five working commands from the registry sweep.** `cdg1_registry_sweep_no_unknowns` enumerates `COMMAND_META` and asserts nothing answers `unknown command`, skipping a list of 10 backlogged-unimplemented names. Five of them had since diff --git a/src/command/metadata.rs b/src/command/metadata.rs index 0b566c1a..c508403d 100644 --- a/src/command/metadata.rs +++ b/src/command/metadata.rs @@ -302,8 +302,6 @@ pub static COMMAND_META: phf::Map<&'static str, CommandMeta> = phf_map! { "OBJECT" => CommandMeta { name: "OBJECT", arity: -2, flags: R, first_key: 2, last_key: 2, step: 1, acl_categories: GEN }, "DBSIZE" => CommandMeta { name: "DBSIZE", arity: 1, flags: RF, first_key: 0, last_key: 0, step: 0, acl_categories: GEN }, "RANDOMKEY" => CommandMeta { name: "RANDOMKEY", arity: 1, flags: R, first_key: 0, last_key: 0, step: 0, acl_categories: GEN }, - "DUMP" => CommandMeta { name: "DUMP", arity: 2, flags: R, first_key: 1, last_key: 1, step: 1, acl_categories: GEN }, - "RESTORE" => CommandMeta { name: "RESTORE", arity: -4, flags: W, first_key: 1, last_key: 1, step: 1, acl_categories: GEN }, "SORT" => CommandMeta { name: "SORT", arity: -2, flags: W, first_key: 1, last_key: 1, step: 1, acl_categories: GEN }, "SORT_RO" => CommandMeta { name: "SORT_RO", arity: -2, flags: R, first_key: 1, last_key: 1, step: 1, acl_categories: GEN }, "COPY" => CommandMeta { name: "COPY", arity: -3, flags: W, first_key: 1, last_key: 2, step: 1, acl_categories: GEN }, @@ -380,10 +378,21 @@ pub static COMMAND_META: phf::Map<&'static str, CommandMeta> = phf_map! { // VACUUM VECTOR | VACUUM GRAPH // Admin + Dangerous category (FREEZE kills all snapshots; treat all variants as dangerous). "VACUUM" => CommandMeta { name: "VACUUM", arity: -1, flags: A, first_key: 0, last_key: 0, step: 0, acl_categories: DNG }, - // MA5: RECLAMATION SCHEDULE — maintenance-window scheduler for autovacuum budget multipliers. - // Arity -3: RECLAMATION SCHEDULE | RECLAMATION SCHEDULE LIST | RECLAMATION SCHEDULE CLEAR - // Admin only (no data mutation, no snapshot impact). - "RECLAMATION" => CommandMeta { name: "RECLAMATION", arity: -3, flags: A, first_key: 0, last_key: 0, step: 0, acl_categories: SRV }, + // DEREGISTERED 2026-08-15 (v0-9-client-compat exit criterion: "no command + // registered here answers unknown command"): DUMP, RESTORE, RECLAMATION, + // MODULE and LATENCY were advertised but dispatched NOWHERE, so COMMAND, + // COMMAND COUNT and ACL all described a surface Moon cannot serve — the + // milestone's own wording was "implement or deregister", and this is the + // deregister half. + // + // Three of the five were never top-level commands at all: `RECLAMATION` is + // reachable only as `DEBUG RECLAMATION`, and `DUMP`/`RESTORE` only as + // `FUNCTION DUMP` / `FUNCTION RESTORE`. Their entries here described + // commands that had no counterpart anywhere in dispatch. + // + // Re-add an entry only together with the arm that serves it — + // `cdg1_registry_sweep_no_unknowns` now enumerates this table with NO + // waiver list, so a registered-but-unreachable command fails the suite. "MEMORY" => CommandMeta { name: "MEMORY", arity: -2, flags: R, first_key: 0, last_key: 0, step: 0, acl_categories: SRV }, "FLUSHDB" => CommandMeta { name: "FLUSHDB", arity: -1, flags: W, first_key: 0, last_key: 0, step: 0, acl_categories: DNG }, "FLUSHALL" => CommandMeta { name: "FLUSHALL", arity: -1, flags: W, first_key: 0, last_key: 0, step: 0, acl_categories: DNG }, @@ -392,8 +401,6 @@ pub static COMMAND_META: phf::Map<&'static str, CommandMeta> = phf_map! { "SHUTDOWN" => CommandMeta { name: "SHUTDOWN", arity: -1, flags: A, first_key: 0, last_key: 0, step: 0, acl_categories: DNG }, "TIME" => CommandMeta { name: "TIME", arity: 1, flags: RF, first_key: 0, last_key: 0, step: 0, acl_categories: SRV }, "LOLWUT" => CommandMeta { name: "LOLWUT", arity: -1, flags: R, first_key: 0, last_key: 0, step: 0, acl_categories: SRV }, - "MODULE" => CommandMeta { name: "MODULE", arity: -2, flags: A, first_key: 0, last_key: 0, step: 0, acl_categories: SRV }, - "LATENCY" => CommandMeta { name: "LATENCY", arity: -2, flags: A, first_key: 0, last_key: 0, step: 0, acl_categories: SRV }, // ---- Pub/Sub commands ---- "SUBSCRIBE" => CommandMeta { name: "SUBSCRIBE", arity: -2, flags: CommandFlags::PUBSUB, first_key: 0, last_key: 0, step: 0, acl_categories: PUB }, diff --git a/tests/wire_reachability_red.rs b/tests/wire_reachability_red.rs index cab0e568..145c1f45 100644 --- a/tests/wire_reachability_red.rs +++ b/tests/wire_reachability_red.rs @@ -268,22 +268,26 @@ fn cdg1_registry_sweep_no_unknowns() { let _guard = ServerGuard(child); drop(wait_ready(port)); - // Backlogged by user decision (contract v2, 2026-06-11 "Fix 26, backlog the - // 10"): advertised in COMMAND_META but implemented NOWHERE. They are - // missing FEATURES (DUMP/RESTORE serialization, latency/module admin), not - // dispatch-routing bugs, and are tracked as an observe-phase delta: - // implement or deregister. + // The waiver list is EMPTY, and stays empty. // - // Shrunk 10 -> 5 (2026-08-15). WATCH, UNWATCH, RESET, SSUBSCRIBE and - // SUNSUBSCRIBE were delivered by the v0-9-client-compat tasks - // `watch-cas-transactions`, `client-identity-introspection` and - // `pubsub-resp3-push`, but were never removed from this list — so for the - // whole milestone this sweep skipped five commands it should have been - // checking, and its green meant less than it appeared to. Measured live - // before removal: WATCH `+OK`, UNWATCH `+OK`, RESET `+RESET`, - // SSUBSCRIBE/SUNSUBSCRIBE a 3-element confirmation. - const BACKLOGGED_UNIMPLEMENTED: &[&str] = - &["LATENCY", "MODULE", "DUMP", "RESTORE", "RECLAMATION"]; + // It held 10 names, waived as "advertised in COMMAND_META but implemented + // nowhere". It reached zero in two steps, both on 2026-08-15: + // + // 10 -> 5 WATCH, UNWATCH, RESET, SSUBSCRIBE, SUNSUBSCRIBE had SHIPPED + // during v0-9-client-compat and nobody removed them, so this + // sweep spent the whole milestone skipping five working + // commands. Measured live: `+OK`, `+OK`, `+RESET`, `*3`, `*3`. + // 5 -> 0 LATENCY, MODULE, DUMP, RESTORE and RECLAMATION were + // DEREGISTERED from COMMAND_META instead — they dispatched + // nowhere, so advertising them made COMMAND and ACL lie about + // the surface. (Three were never top-level commands at all: + // `DEBUG RECLAMATION`, `FUNCTION DUMP`, `FUNCTION RESTORE`.) + // + // So the sweep now covers EVERY entry in the registry with no exemptions, + // which is what the milestone exit criterion actually asked for. Do not + // reintroduce this list: an unreachable command is a registry bug, and the + // fix is to remove the entry or add the dispatch arm, not to waive it. + const BACKLOGGED_UNIMPLEMENTED: &[&str] = &[]; let mut violations: Vec = Vec::new(); for name in moon::command::metadata::COMMAND_META.keys() {