extend-commands-api

# Extend the Jedis Commands API Implement a new Redis command (or extend an existing one) in Jedis, following the conventions Jedis maintainers enforce in review. ## Modes — read this first This skill runs in one of two modes. The engineering rules in every section below are identical in both; only who answers questions and approves the plan differs. - **Supervised** (default): a human is in the session. Ask the questions, present the plan in plan mode and wait for approval, as written below. - **Unattended**: no human will answer or approve anything, so never stop to wait. Use it ONLY when the invoking prompt says `Mode: unattended` or the environment has `CLIENT_SKILL_MODE=unattended`. Never switch to it on your own. In unattended mode the invoking prompt supplies the inputs a human would give: the HLD path (normally `./HLD.md`), the server PR reference, the test command, and two running servers from the `redislabs/client-libs-test` image it names, password- protected like the local Docker env and without TLS: a standalone (`$REDIS_URL`, password `$REDIS_STANDALONE_PASSWORD`, i.e. `foobared`) and a 6-node cluster with 3 masters and 3 replicas (`$REDIS_CLUSTER_URLS`, password `$REDIS_CLUSTER_PASSWORD`, i.e. `cluster`). `$REDIS_ENDPOINTS_CONFIG_PATH` already points at an `endpoints.json` that describes them, passwords included, as `standalone0`, `modules-docker` (the standalone; the image ships the modules) and `cluster-stable`. Jedis tests read that variable (`TestEnvUtil.getEndpointsConfigPath`) and authenticate with the passwords in it, so the integration tests run against both servers unchanged. Wherever a step below has an **Unattended:** note, follow the note: | Step | Supervised | Unattended | |---|---|---| | Phase 0.1 HLD | ask the user for the path | read the given HLD fully | | Phase 0.2 server PR | `gh`, asking to run it outside the sandbox if needed | no `gh` at all: the given PR reference, the HLD's facts, else the unauthenticated GitHub REST API | | Phase 0.3 environment / image tag | `make start`; ask for an image tag if the command is missing | no Docker: probe the given standalone and cluster; if the command is missing there, continue with gated tests | | Phase 0.4 redis-cli scenarios | run against `standalone0` | run against the given Redis URL; if `redis-cli` is unavailable, write them as unverified transcripts | | Phase 1 plan + approval | plan mode, ask before editing | write the plan into the final report, then implement it in the same session | | Running tests | `make start`, `mvn -B verify`, `make stop` | `mvn -B test` (unit), then the changed family's integration tests on standalone **and** cluster through `$REDIS_ENDPOINTS_CONFIG_PATH` (see Running the tests) | | An open design question | ask the user | take the HLD's choice (else the most consistent existing Jedis convention) and list it as an open question in the report | Unattended runs also follow these rules: - **Change files.** A run that ends without changes has failed. Stop without editing only when implementing is impossible, and say exactly why in the report. - **Don't commit, push, or open a PR.** The automation that invoked you does that. Don't edit `.github/` workflows, release/version files or `pom.xml` versions. The formatter includes in `pom.xml` are the exception (see B.7). - **Finish with a report** covering, in plain text with no placeholders: - what was implemented - the decision-tree class (A–D) and the plan - design decisions, and which layers intentionally did NOT change and why - unit results; integration results with the test classes run on standalone and on cluster; integration tests left gated, skipped, or out of scope - steps skipped because they need a human or Docker - open questions for maintainers This feeds the PR description (see the PR hygiene checklist). ## Phase 0 — Gather evidence BEFORE planning Do all of the following before writing any plan or code: 1. **Ask the user for the HLD.** Interactively ask the user for the path to a markdown file containing the High-Level Design for the command(s) (or confirm that no HLD exists). If a path is given, read it fully — it is the primary source for syntax, semantics, reply shape, and edge cases. **Unattended:** don't ask; read the HLD path the invoking prompt gives. 2. **Find the server-side PR in the `redis/redis` GitHub repo.** **Unattended:** skip every `gh` command in this step: `gh auth status`, and the `gh search` / `gh pr view` / `gh pr diff` block below. Don't ask for permission. Use the server PR reference the invoking prompt gives. The HLD usually quotes the syntax, the reply shapes and the first version. Fill any gaps from the unauthenticated GitHub REST API (e.g. `https://api.github.com/repos/redis/redis/pulls/<num>/files`, then the raw `src/commands/*.json` file), and go on to "From the server PR, extract". **Supervised:** first verify that `gh` works in the current (sandboxed) environment: ```bash gh auth status ``` Sandboxes often block access to credential files (`~/.netrc`, gh config), so `gh` may report unauthenticated here even though it works on the user's machine. If `gh auth status` fails or reports no authentication, **ask the user for permission to run the `gh` commands outside the sandbox** (e.g. with the sandbox override for these specific read-only commands), explaining that `gh` cannot reach its credentials from inside the sandbox. Only if the user declines (or `gh` is genuinely not logged in anywhere) fall back to the unauthenticated GitHub REST API or fetching `https://github.com/redis/redis/pulls?q=<command>`. Then search for the PR that adds/extends the command on the server: ```bash gh search prs --repo redis/redis "<COMMAND NAME>" --limit 10 gh pr view <num> --repo redis/redis gh pr diff <num> --repo redis/redis # look at src/commands/*.json for exact syntax ``` From the server PR, extract: exact wire syntax (argument order and optionality), reply type per RESP2/RESP3, error conditions, time complexity, and — critically — the **first server version carrying the feature** (RC builds like `8.7.225` = Redis 8.8 RC are used for test gating). Note whether the server marks the feature *experimental / in preview*. 3. **Verify the command exists on the "next" Redis OSS version.** Start the integration environment using the **latest** `.env.vX.XX` file under `src/test/resources/env/` (pick the highest version numerically, not lexicographically — e.g. `8.10` > `8.8`): ```bash ls src/test/resources/env/.env.v* | sort -V | tail -1 # → e.g. .env.v8.10 make start version=8.10 ``` Then probe the `standalone0` endpoint. Read its connection details from `src/test/resources/endpoints.json` (currently `redis://localhost:6379`, password `foobared`) and check the target command via redis-cli: ```bash redis-cli -u redis://localhost:6379 -a foobared INFO server | grep redis_version redis-cli -u redis://localhost:6379 -a foobared COMMAND INFO <COMMAND> # non-empty → command exists redis-cli -u redis://localhost:6379 -a foobared COMMAND DOCS <COMMAND> # arity/args — compare with the server PR ``` For a NEW command, `COMMAND INFO` must return a non-empty reply. For an EXTENDED command, additionally invoke it with the new syntax against a scratch key and confirm the server accepts it (no `ERR syntax error` / `ERR unknown argument`). **If the command/option is missing on the latest env version**, the image tag pinned in the `.env.vX.XX` file is too old for the target change. Interactively **ask the user for a `redislabs/client-libs-test` image tag** that contains it (e.g. an RC/edge/milestone build — tags are listed at https://hub.docker.com/r/redislabs/client-libs-test/tags; you may check them yourself and suggest candidates). Then restart the environment with that tag and re-run the probes: ```bash make stop make start CLIENT_LIBS_TEST_IMAGE_TAG=<tag> ``` If the user has no tag to offer (or the probe still fails), report it and proceed anyway: the implementation can continue, but integration tests will be gated/skipped until a `redislabs/client-libs-test` image ships the change. Keep the environment running for later test runs, or `make stop` if you won't need it soon. **Unattended:** no Docker, no `make start`/`make stop`, no image-tag question. The servers already run the target image. Probe the standalone with the same `INFO server` / `COMMAND INFO` / `COMMAND DOCS` checks (e.g. `redis-cli -u "$REDIS_URL" COMMAND INFO <COMMAND>`; the URL carries the credentials), and the cluster with `redis-cli -c -h "$REDIS_CLUSTER_HOST" -p "$REDIS_CLUSTER_START_PORT" -a "$REDIS_CLUSTER_PASSWORD" --no-auth-warning CLUSTER INFO` (expect `cluster_state:ok`). If the command is missing, continue anyway with gated integration tests, and say so in the report. 4. **Create redis-cli showcase test cases.** Once the command is available on the running environment, derive a small set of redis-cli scenarios from the HLD and the server PR and run them against `standalone0`. These serve two purposes at once: - **Smoke test**: prove the documented behavior actually holds on the target server — happy path, each new option/keyword, reply shape (run with `redis-cli -3` too if RESP3 replies differ), edge cases and error conditions called out in the HLD (empty/missing key, out-of-range args, conflicting options), so the Java implementation is built against observed replies, not assumptions. - **Showcase**: each scenario should read as a mini use-case that explains WHY the command/option exists — what a user could not do (or could only do awkwardly) before, and how the new API covers it. Prefer realistic data over `foo`/`bar` (e.g. rate-limit counters for a bounded INCR, sensor readings for a time-series aggregator). Save the scenarios as a commented script in a scratch file (one block per use-case: a one-line "what this demonstrates" comment, the redis-cli commands, and the observed reply pasted back as comments). Carry this material forward: it feeds the Phase 1 plan (as the explanation of the feature and the source of expected values for tests), the Java test assertions, and the PR description. If the command could not be made available on any image (see step 3), still write the scenarios from the HLD/PR as *expected* transcripts and mark them unverified. **Unattended:** run the scenarios against `$REDIS_URL` instead of `standalone0`. For a keyed command, also run one through the cluster. Always pass the Redis command as arguments: a bare `redis-cli` waits on stdin and hangs a headless run. ```bash redis-cli -u "$REDIS_URL" <COMMAND> <args...> redis-cli -c -h "$REDIS_CLUSTER_HOST" -p "$REDIS_CLUSTER_START_PORT" \ -a "$REDIS_CLUSTER_PASSWORD" --no-auth-warning <COMMAND> <args...> ``` If `redis-cli` isn't installed, write them as unverified expected transcripts. Keep the scratch file out of the change; carry the scenarios into the report instead. 5. **Read `docs/integration-testing.md`** in the Jedis repo — it defines the test environment, endpoint discovery, `*Test` vs `*IT` naming, and how to run tests. 6. **Trace one analogous existing command** end-to-end in the codebase (same command group, similar reply shape) so the plan mirrors real code, not guesswork. 7. Determine the `@since` version: ```bash mvn help:evaluate -Dexpression=project.version -q -DforceStdout ``` Strip `-SNAPSHOT` (e.g. `8.0.0-SNAPSHOT` → `@since 8.0`). ## Phase 1 — Plan, then approval (supervised) or straight to implementation (unattended) Using the evidence, classify the change with the decision tree below, enumerate the exact file-by-file touch list, the test matrix, and the gating annotations. Open the plan with a short "what this feature enables" section built from the redis-cli showcase scenarios (Phase 0 step 4), including one or two representative command/reply transcripts, so the reader sees the use-case before the file list. - **Supervised:** enter **plan mode**, present the plan to the user and **explicitly ask permission to execute in auto-accept mode** before implementing. Do not start editing files until the user approves. - **Unattended:** don't enter plan mode and don't wait. The automation's approval was the merged HLD. Keep the plan for the final report, then implement it right away, following the whole matrix exactly as a supervised run would. ## Decision tree — what kind of change is this? **A. Extension of an existing command that fits an existing params class** (e.g. new enum value / new option token): - Touch ONLY the params/model classes + tests. Do NOT touch command interfaces, CommandObjects, UnifiedJedis, PipeliningBase, or Jedis — the existing `IParams.addParams(CommandArguments)` delegation carries the new option through automatically, for both String and binary surfaces. - If the option is a bare token: add an enum constant implementing `Rawable` (bytes come from `SafeEncoder.encode(name())` — self-wiring). - If it's a new field: add a fluent setter returning `this`, extend `addParams()`, and **update `equals`/`hashCode`** in sync. - If the reply shape grows (e.g. more array elements): extend the response model class backward-compatibly — keep old constructors/getters working, add new accessors, update the builder in the relevant `*BuilderFactory` to branch on reply size. Jedis does not defensively copy response collections (stated maintainer convention). **B. New core command(s)** — the FULL matrix, every layer: 1. `Protocol.java` — new `Command` enum constant(s); new `Keyword` constants for sub-tokens, grouped under a `// <FEATURE> keywords` comment, alphabetical. **Never add a Keyword that duplicates a token already carried by a dedicated `Rawable` enum** (dead keywords get flagged in review). 2. Command interfaces in `commands/`: `<Group>Commands`, `<Group>BinaryCommands`, `<Group>PipelineCommands`, `<Group>PipelineBinaryCommands`. For a whole new command *family*, create four new `<Family>*Commands` interfaces and add one `extends` entry to `JedisCommands`, `JedisBinaryCommands`, `PipelineCommands`, `PipelineBinaryCommands`. 3. `CommandObjects` — String and byte[] factory methods side by side under a `// <Feature> commands` section. `ClusterCommandObjects` needs overrides ONLY for multi-key commands requiring slot checks; single-key commands need nothing. 4. `UnifiedJedis` — one-line `@Override` delegating to `executeCommand(commandObjects.xxx(...))`. 5. `PipeliningBase` — `appendCommand(commandObjects.xxx(...))` returning `Response<T>`. 6. `Jedis` (legacy) — `checkIsInMultiOrPipeline(); connection.executeCommand(commandObjects.xxx(...))`. 7. `pom.xml` — add every new file to the **formatter-maven-plugin includes** list (this repo format-enforces an allowlist; new files must be registered). **C. Extension needing new overloads / a new params class** (hybrid): new methods go through the full matrix of B; the option plumbing follows the params conventions below. **D. Module command (search / timeseries / json / bloom)**: - Module interfaces are **String-only** — there are NO `*BinaryCommands` variants for modules; never create them. - Module commands/keywords live in the module protocol class (`TimeSeriesProtocol.TimeSeriesCommand/TimeSeriesKeyword`, `SearchProtocol.SearchCommand/SearchKeyword`, …), each an enum implementing `ProtocolCommand`/`Rawable` with `SafeEncoder.encode()`-cached bytes. - Strongly prefer extending the module's params/builder classes over adding interface methods — PRs #4504 and #4534 shipped whole features with ZERO changes to interfaces, CommandObjects, UnifiedJedis, or PipeliningBase. - Search aggregation builders (`Reducer` subclasses) build a `List<Object>` via `getOwnArgs()`; the parent computes `narg` automatically. Canonicalize clause order at serialization time regardless of builder call order. - Module response parsing lives in the module's builder factory (`TimeSeriesBuilderFactory`, `SearchBuilderFactory`); search aggregation results are loosely typed and often absorb new reply content with no parsing changes. ## Binary (byte[]) variant policy - **Core commands: full String/byte[] parity is mandatory** — all four interfaces, paired CommandObjects methods, and all three clients. Reviewers explicitly request binary integration tests too. - Only **keys and textual values** get `byte[]` overloads. Numeric/structural args (`long index`, ranges, booleans, enums) stay identical. - **Params classes are shared, never duplicated**: one params class serves both surfaces; if a param carries text, give the params class both `foo(String)` and `foo(byte[])` setters (see `ArgrepParams`). - Return types mirror the key type (`String`→`byte[]`, `List<String>`→`List<byte[]>`); count/index/model results stay shared. - **Modules: no binary variants at all.** ## Params class conventions (`params/`) - Implement `IParams` with `addParams(CommandArguments args)`. Fluent setters return `this`; provide a static factory named after the class (`increxParams()`, `rangeParams()`). - Do NOT extend another command's params hierarchy. If two overloads share options but differ in a typed field (long vs double), create a self-typed abstract base: `BaseFooParams<T extends BaseFooParams<T>>` with a private `self()` cast, plus two concrete subclasses — compile-time type safety over a polymorphic single class. - Implement **`equals`/`hashCode`** (needed for Mockito matching in mocked tests) and keep them in sync with every new field. `toString` is not required on params classes. - Validate eagerly in setters with descriptive `IllegalArgumentException` / `IllegalStateException` messages, consistent wording across sibling classes (e.g. "Aggregators must be non-null and non-empty"). Required-config checks may run at serialization time. No client-side server-version checks — an old server returning an error is acceptable and should be noted in the PR description. - Expiration-style option groups are single-slot, last-wins (mirror `SetParams`). - Document the wire-emission order in javadoc and lock it down in unit tests. ## Response mapping (`BuilderFactory` / `resps/`) - **Reuse existing generic builders whenever the reply maps to a standard shape** (`LONG_LIST`, `DOUBLE_LIST`, `STRING`, `ENCODED_OBJECT_MAP`, …). Maintainers actively reject bespoke response classes for 2-element arrays and the like. - Reuse `redis.clients.jedis.util.KeyValue<K,V>` for pair replies.