extend-commands-api

# Extend the NRedisStack Commands API Implement a new Redis command (or extend an existing one) in NRedisStack, following the conventions maintainers enforce in review. Work evidence-first: prove the command's behavior on a real server before designing the C# API. ## 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/module PR reference (`tracks:`), the test command (`dotnet test tests/NRedisStack.Tests/NRedisStack.Tests.csproj -f net8.0`), and a running standalone Redis plus a 6-node OSS cluster (3 primaries + 3 replicas) from `redislabs/client-libs-test` (modules included, password-protected, no TLS, no Docker available to you). The environment carries `REDIS_URL` (with credentials), `REDIS_STANDALONE_HOST`/`REDIS_STANDALONE_PORT`/`REDIS_STANDALONE_PASSWORD`, `REDIS_CLUSTER_HOST`, `REDIS_CLUSTER_START_PORT`, `REDIS_CLUSTER_NODES`, `REDIS_CLUSTER_PASSWORD`, `REDIS_CLUSTER_URLS`, `REDIS_VERSION`, and `REDIS_ENDPOINTS_CONFIG_PATH` (an `endpoints.json` already in NRedisStack's format with ids `standalone` and `cluster`, each with its `password`, which `EndpointConfig.CreateConnection` applies). 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 `gh` access | ask to run `gh` outside the sandbox | do not ask; use the `tracks:` PR and HLD facts, else the unauthenticated GitHub REST API | | Phase 0.3 environment / image tag | `docker compose ... up -d --wait`; ask for an image tag if the command is missing | no Docker: probe `$REDIS_URL` and the cluster; if the command is missing, continue with version-gated tests | | Phase 0.4 redis-cli scenarios | run against `127.0.0.1:6379` | run against `$REDIS_URL` (and `-c` on the cluster); if `redis-cli` is unavailable, write 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 | human starts compose as CI does; `dotnet test` with `--filter` | `dotnet test` with `--filter` against BOTH `standalone` and `cluster`, using the given `REDIS_ENDPOINTS_CONFIG_PATH` and `REDIS_VERSION` | | An open design question | ask the user | take the HLD's choice (else the most consistent existing NRedisStack convention) and list it as an open question in the report | | Experimental (`[Experimental]`) or not | ask if the HLD is unclear | experimental only if the HLD/server PR says preview / unstable-features-gated; otherwise stable | Unattended runs also follow these rules: - **Change files.** A run that ends without changes has failed. Stop without editing only when implementing is impossible (or the command belongs to StackExchange.Redis, see decision D), and say exactly why in the report. - **Don't commit, push, or open a PR.** The automation that invoked you does that. - **No CI / release / version edits**: nothing under `.github/`, no `tests/dockers/.env.v*` or `docker-compose.yml` image bumps, no `version.json` or `<Version>` changes, no `PublicAPI.Shipped.txt` promotion. Editing `src/NRedisStack/PublicAPI/PublicAPI.Unshipped.txt` IS required for every new public member. For a new experimental id only, the `Directory.Build.props` `NoWarn` entry is the one allowed build-file edit (see "Experimental API"). - **Finish with a plain-text report** (no placeholders) covering: - what was implemented, the decision-tree class (A-E) and the plan - design decisions, and which layers intentionally did NOT change and why - the command category chosen for every new/changed builder method, and why - unit test results, and integration results per target: the `--filter` used, passed/failed/skipped counts, test names, and confirmation that both the `standalone` and `cluster` rows ran (any skip with its reason) - 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 1. **Ask the user for the HLD.** Ask for the path to a markdown file with the High-Level Design (or confirm none exists). Read it fully - it is the primary source for syntax, semantics, reply shape per RESP2/RESP3, and edge cases. **Unattended:** don't ask; read the HLD path the invoking prompt gives. 2. **Find the server-side PR in the repo that owns the command family:** | Command family | Repository | |---|---| | Core commands | `redis/redis` (`src/commands/*.json` has the exact syntax) | | Search / query engine (`FT.*`) | `RediSearch/RediSearch` | | JSON (`JSON.*`) | `RedisJSON/RedisJSON` | | Probabilistic (`BF.*`, `CF.*`, `CMS.*`, `TOPK.*`, `TDIGEST.*`) | `RedisBloom/RedisBloom` | | Time series (`TS.*`) | `RedisTimeSeries/RedisTimeSeries` | If the routed repo has nothing, try `redis/redis` (and vice versa). Check `gh auth status` first; sandboxes often hide gh's credentials, so if it fails ask the user for permission to run these read-only `gh` commands outside the sandbox, and only fall back to the unauthenticated REST API if they decline. ```bash gh search prs --repo <owning-repo> "<COMMAND>" --limit 10 gh pr view <num> --repo <owning-repo> gh pr diff <num> --repo <owning-repo> ``` Extract: exact wire syntax (argument order, optionality), reply type per **RESP2 and RESP3** (module replies often become maps in RESP3), error conditions, whether the command is keyed / keyless / node-bound, and the **first server version carrying it** (RC builds such as `8.3.224` = 8.4 RC are valid gates). Note if the feature is *preview* or gated behind `search-enable-unstable-features` and the like. **Unattended:** don't ask for permission. Use the `tracks:` reference and the HLD; fill gaps from `https://api.github.com/repos/<owner>/<repo>/pulls/<num>/files`. 3. **Verify the command exists on the target server.** CI starts both topologies from `tests/dockers/docker-compose.yml`, selecting the image via `tests/dockers/.env.v<X.Y>` (pick the highest numerically: `8.10` > `8.8`): ```bash ls tests/dockers/.env.v* | sort -V | tail -1 docker compose --env-file tests/dockers/.env.v8.10 --profile all \ -f tests/dockers/docker-compose.yml up -d --wait redis-cli -p 6379 INFO server | grep redis_version redis-cli -p 6379 MODULE LIST # module present + version redis-cli -p 6379 COMMAND INFO <COMMAND> # non-empty -> exists redis-cli -p 6379 COMMAND DOCS <COMMAND> # args, compare with the PR ``` Standalone is `127.0.0.1:6379`; the cluster is `127.0.0.1:16379-16384` (see `tests/dockers/endpoints.json`). For an EXTENDED command, also run the new syntax against a scratch key and confirm no `ERR syntax error` / `unknown argument`. **If it is missing**, the pinned image is too old. Ask the user for a `redislabs/client-libs-test` tag that has it (tags: https://hub.docker.com/r/redislabs/client-libs-test/tags - you may suggest candidates), then restart with `CLIENT_LIBS_TEST_IMAGE=redislabs/client-libs-test:<tag> docker compose --profile all -f tests/dockers/docker-compose.yml up -d --wait` (compose reads the full image reference). CI's custom-image job runs such images with `REDIS_VERSION=255.255.255`. If no image has it, report that and proceed: the tests will be written and gated, not run. **Unattended:** no Docker, no image question. Probe what you were given: `redis-cli -u "$REDIS_URL" COMMAND INFO <COMMAND>` and `redis-cli -c -h "$REDIS_CLUSTER_HOST" -p "$REDIS_CLUSTER_START_PORT" -a "$REDIS_CLUSTER_PASSWORD" --no-auth-warning COMMAND INFO <COMMAND>`. If missing there, continue with version-gated tests and say so in the report. 4. **Create redis-cli showcase scenarios** from the HLD and the PR, and run them: - **Smoke test**: happy path, each new option, reply shape under `redis-cli -3` as well as RESP2, edge cases and errors (missing key/index, out-of-range args, conflicting options) - so parsing is built on observed replies. For keyless or multi-key commands, run them on the cluster too (`-c`); fan-out and CROSSSLOT behavior differs from standalone. - **Showcase**: each scenario reads as a mini use-case explaining WHY the command/option exists. Prefer realistic data over `foo`/`bar`. Keep them in a scratch file (a "what this demonstrates" comment, the commands, the observed reply as comments). They feed the plan, the test assertions and the PR description. If the command was unavailable, write them as *expected* transcripts and mark them unverified. **Unattended:** run against `$REDIS_URL` / the cluster. Keep the scratch file out of the change; carry the scenarios into the report. 5. **Trace one analogous existing command end to end** (same module, similar reply shape). Reference commits, validated against this repo's history: | Commit | What it exemplifies | |---|---| | `37f7662` (FT.ALIASLIST #518) | Minimal new module command: literal, builder, interface x2, impl x2, PublicAPI, sync+async theories gated `8.10.0` | | `f639686` (TS 8.10 #528) | New commands + new options on existing ones: `[Flags]` enum, new overloads with the old ones kept `[Obsolete]` for binary compat, `(RedisKey)` casts for cluster routing, per-command test files | | `d5644f5` (CMS.INFO cell size #554) | Reply gained a field: new DataTypes property (`-1` on older servers) + parser case | | `62e2d19` + `25f01e4` (COLLECT #524, NRS003 #536) | Option via a builder type (no interface change), then marked `[Experimental]` | | `87ea197` (#540) | Why every `SerializedCommand` declares a command category | Older code can predate current rules (for example, pre-#540 diffs construct `SerializedCommand` without a category). When precedent conflicts with this skill, the written convention wins. ## Phase 1 - Plan, then approval (supervised) or straight to implementation (unattended) Classify the change with the decision tree, then list: the file-by-file touch list, the proposed public signatures (sync + async) with XML docs, the command category per builder method, the PublicAPI lines, the test matrix and the version gate. Open with a short "what this feature enables" section with one or two showcase transcripts. - **Supervised:** enter **plan mode**, present the plan and **explicitly ask permission to execute** before editing. The public signature is the contract; if it must change during implementation, stop and re-confirm. - **Unattended:** don't enter plan mode and don't wait. The merged HLD was the approval. Keep the plan for the final report and implement it right away, following the whole matrix exactly as a supervised run would. ## Decision tree - what kind of change is this? **A. New option / argument on an existing command.** - If the command already takes a parameter object, extend THAT and nothing else: Search `Query`, `FTCreateParams`, `FTSpellCheckParams`, `AggregationRequest` / `Reducers`, `HybridSearchQuery`; TimeSeries `TimeSeriesParamsBuilder` (`TsAddParamsBuilder`, ...); a `[Flags]` enum such as `TimeSeriesRangeFlags`. The builder already serializes the object, so interfaces and impls don't change (cf. COLLECT). Add the token to `Literals/CommandArgs.cs` and keep wire order. - If the option is a positional method parameter, **do not edit the existing signature** (adding even an optional parameter breaks binary compatibility and changes a shipped PublicAPI line). Add a new overload carrying the option; keep the old one, marked as in `ITimeSeriesCommands.Range`: `[Obsolete] [Browsable(false)] [EditorBrowsable(EditorBrowsableState.Never)] [OverloadResolutionPriority(-1)]`, with `[OverloadResolutionPriority(1)]` on the new one, so existing calls resolve to the new overload. Mirror on sync + async interfaces, both impls and the builder. Where an integer literal could bind to the obsolete overload, add a compile-time guard call like `TimeSeries/TestAPI/ObsoleteOverloadResolutionGuard.cs`. - Re-check the command category: an option can change it (JSON.SET NX/XX, FT.SUGADD INCR, TS.ADD ON_DUPLICATE) - pin those in `CommandCategoryTests`. **B. Reply gained fields (INFO-style).** Extend the `DataTypes/` model backward-compatibly: new get-only property, default for older servers (CMS used `-1`), constructor stays `internal`; add the `case` in the parser in `ResponseParser.cs` for RESP2 and, if it differs, RESP3 (`Resp3Type is ResultType.Map`). **C. New command in an existing module** - the FULL matrix (FT.ALIASLIST): 1. `src/NRedisStack/<Module>/Literals/Commands.cs` - `public const string ALIASLIST = "FT.ALIASLIST";` in the module's `internal class` (`FT`, `JSON`, `TS`, `BF`, `CF`, `CMS`, `TOPK`, `TDIGEST`); sub-tokens in `Literals/CommandArgs.cs` (`SearchArgs`, ...). 2. `<Module>CommandBuilder.cs` (`TimeSeriesCommandsBuilder.cs` for TS) - a `public static SerializedCommand` method, args in wire order, with a category: ```csharp public static SerializedCommand AliasList(string index) { return new(CommandCategories.ReadOnly, FT.ALIASLIST, index); } ``` 3. `I<Module>Commands.cs` and `I<Module>CommandsAsync.cs` - the documented contract (see Docs below); async name = sync name + `Async`, returning `Task<T>`. 4. `<Module>CommandsAsync.cs` - `/// <inheritdoc/>` then `return (await _db.ExecuteAsync(SearchCommandBuilder.AliasList(index))).ToArray();` 5. `<Module>Commands.cs` (the sync class inherits the async class) - `/// <inheritdoc/>` then `return db.Execute(SearchCommandBuilder.AliasList(index)).ToArray();` 6. `src/NRedisStack/PublicAPI/PublicAPI.Unshipped.txt` - one line per public member: both interface members, both concrete members, the static builder. 7. Parsing: reuse `ResponseParser` (`ToArray`, `ToLong`, `ToDouble`, `OKtoBoolean`, `ToBooleanArray`, `ToStringList`, `ToStringRedisResultDictionary`, ...). Add a new `internal` extension there (or in `<Module>Aux.cs`) and a `DataTypes/` class only when the reply is genuinely structured. 8. Pipelines/transactions need no wiring: `Pipeline` and `Transaction` expose the `*CommandsAsync` classes, so the async method is the batch API. **D. New core (non-module) command.** First check StackExchange.Redis: if `IDatabase` already has it (or will, in the referenced SE.Redis version), NRedisStack does not wrap it - report that instead of duplicating it. NRedisStack only covers core gaps (blocking pops, `XREAD`/`XREADGROUP` blocking, `CLIENT SETINFO`). Those are `public static` **extension methods on `IDatabase`** in `CoreCommands/CoreCommands.cs` + `CoreCommandsAsync.cs` (no interface; XML docs go on the methods), built in `CoreCommandBuilder.cs`, with literals in `CoreCommands/Literals/` (`RedisCoreCommands`, `CoreArgs`), types in `CoreCommands/DataTypes` / `Enums`, tests in `tests/NRedisStack.Tests/Core Commands/CoreTests.cs`. **E. New module / command group** (rare): mirror a whole module folder, add the `db.XX()` accessor in `ModulePrefixes.cs` and a property in both `Pipeline.cs` and `Transactions.cs`. Ask first (supervised) / list as an open question (unattended). ## Conventions (verified in the code) **Command category (mandatory).** Since #540 every builder passes a `CommandFlags` category as the first `SerializedCommand` argument, using the aliases in `RedisStackCommands/CommandCategories.cs`. The uncategorized ctors