From cacfac4d41276604dfeeea1db25d8a16370f1ce6 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 16:52:50 +0000 Subject: [PATCH 1/2] docs(service-cluster-redis): record the measured ioredis 5-vs-6 gap in the contract test header, and drop a promised live-Redis path that never existed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The contract test runs `ioredis-mock@8.13.1` (peer `ioredis: ^5`) against a package depending on `ioredis@^6`, with the untyped import suppressed by `@ts-expect-error`. This commit changes no code and no dependency: it writes down the measurement that was missing, so the next reader starts where this one finished. Measured against ioredis 5.11.1 (newest release satisfying the mock's `^5` peer) and 6.0.0 (the version resolved in this workspace): - all 14 Redis commands this package issues carry every v5 overload verbatim into v6's RedisCommander.d.ts; `set` is a strict superset there (v6 adds IFEQ/IFNE/IFDEQ/IFDNE), and nothing used here was removed or re-shaped - v6's RESP3 reply mapping is opt-in: the class defaults its ReplyMapping parameter to "legacy", ChainableCommander defaults to "resp2", and duplicate() with no override inherits the caller's mapping. This package never passes replyMapping, so every reply shape it sees is the v5 one - the three RedisOptions keys client.ts sets (lazyConnect, maxRetriesPerRequest, enableAutoPipelining) are declared identically The peer mismatch is therefore real but inert on the surface these suites drive. The header now says so, names the version pair the claim rests on, and states that the claim expires if either range in package.json moves. Also recorded: these suites reach the mock only through the injected client and the pub/sub duplicate, never through createRedisClient(), so `new Redis(url, options)` — this package's only contact with ioredis's constructor and connection surface, and the area v6 changed most — is exercised by nothing in this file. That is why the gap is inert here, and it is not a reason to trust the double. The header additionally promised `RUN_REAL_REDIS=1` + `REDIS_URL` and "conditional describe.skipIf blocks at the bottom". No such blocks exist, and both names occurred nowhere in the package outside that one sentence. The false promise is removed rather than left standing as an escape hatch nobody can take. The `@ts-expect-error` stays. Removing it fails typecheck with TS7016 — ioredis-mock ships no `types` field and no .d.ts of its own — so the suppression has not rotted. Its comment now also records what it costs: with the module untyped the client is `any` and satisfies ioredis's Redis type without being checked against it, which is the second reason a v5-vs-v6 divergence could not surface here. `@types/ioredis-mock` would type the seam but only asserts `new(): ioredis.Redis` rather than describing the mock, so adopting it would trade an honest `any` for an unearned certainty; that trade is left to a maintainer. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --- .../src/redis.contract.test.ts | 61 ++++++++++++++++++- 1 file changed, 58 insertions(+), 3 deletions(-) diff --git a/packages/services/service-cluster-redis/src/redis.contract.test.ts b/packages/services/service-cluster-redis/src/redis.contract.test.ts index 5b955a43de..b015f34f3f 100644 --- a/packages/services/service-cluster-redis/src/redis.contract.test.ts +++ b/packages/services/service-cluster-redis/src/redis.contract.test.ts @@ -3,12 +3,67 @@ /** * Driver contract tests for the Redis cluster driver, run against * `ioredis-mock` so they execute without a real Redis instance in CI. + * There is no live-Redis path in this file: every suite below runs on the + * mock. (An earlier version of this header promised `RUN_REAL_REDIS=1` + + * `REDIS_URL` and "conditional `describe.skipIf` blocks at the bottom" — + * no such blocks were ever here, and the two names appeared nowhere in + * this package outside that sentence.) * - * The same suites can be invoked against a live Redis by setting - * `RUN_REAL_REDIS=1` and providing `REDIS_URL` — see the conditional - * `describe.skipIf` blocks at the bottom. + * ## The double is one major version behind the client it doubles + * + * This package depends on `ioredis@^6`, while `ioredis-mock@8.13.1` + * declares `peerDependencies: { ioredis: "^5" }` — so `pnpm install` + * prints an unmet-peer warning for it. That gap is real, and it is + * declared here rather than closed, because it was measured to be inert + * on the surface these suites actually drive. Measured against ioredis + * 5.11.1 (newest release satisfying the mock's `^5` peer) and 6.0.0 (the + * version resolved in this workspace): + * + * - Every Redis command issued by this package's `src/*.ts` — get, set, + * del, incr, incrby, pttl, watch, unwatch, multi, exec, publish, + * subscribe, unsubscribe, quit, eval — carries all of its v5 + * overloads verbatim into v6's `RedisCommander.d.ts`. `set` is a + * strict superset there (v6 adds the IFEQ/IFNE/IFDEQ/IFDNE tokens); + * nothing used here was removed or re-shaped. + * - v6's one substantive change reachable from this package is RESP3 + * reply mapping, and it is opt-in: the class is declared with a + * `ReplyMapping` parameter defaulting to "legacy", `ChainableCommander` + * defaults to "resp2", and `duplicate()` with no override inherits the + * caller's mapping. This package never passes `replyMapping`, so every + * reply shape it sees is still the v5 one. + * - The three `RedisOptions` keys client.ts sets — lazyConnect, + * maxRetriesPerRequest, enableAutoPipelining — are declared + * identically in both versions. + * + * That named set is the whole basis for the claim; it is not a statement + * about ioredis 5 vs 6 in general. If the `ioredis` or `ioredis-mock` + * range in package.json moves, this paragraph expires and the diff has to + * be re-taken. + * + * ## What these suites therefore do NOT certify + * + * They reach the mock only through the injected `client` and the + * `client.duplicate()` the pub/sub adapter makes. They never call + * `createRedisClient()`, so `new Redis(url, options)` — this package's + * only contact with ioredis's constructor and connection surface, and the + * area v6 changed most — is exercised by nothing in this file. That is + * why the version gap is inert here, and it is not a reason to trust the + * double: the mock is simply never asked to stand in for the surface on + * which the two majors differ. */ +// `ioredis-mock` publishes no type declarations of its own — no `types` +// field in its manifest and no `.d.ts` in the tarball — so this import +// raises TS7016 ("could not find a declaration file ... implicitly has an +// 'any' type") and the directive below is what silences it. Note the +// second cost, beyond the missing types: with the module untyped +// `RedisMock` is `any`, so every `client:` argument constructed from it +// satisfies ioredis's `Redis` type without ever being checked against it. +// That is the other reason a v5-vs-v6 divergence could not surface here. +// `@types/ioredis-mock` exists and would type this seam, but it only +// *asserts* `new(): ioredis.Redis` rather than describing the mock, so +// adopting it would trade an honest `any` for an unearned certainty — +// that trade has not been made, deliberately. // @ts-expect-error — ioredis-mock has no published types import RedisMock from 'ioredis-mock'; import { describe, expect, it, vi } from 'vitest'; From a2bbec441d06d5604ceb162eafe98b89b52087e9 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 17:45:25 +0000 Subject: [PATCH 2/2] docs(service-cluster-redis): narrow three contract-test header sentences to the populations actually measured MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found three sentences in the header whose stated population was wider than the measurement behind them. All three are corrected here; the diff stays comments-only (proved: every added and removed line is a comment line, with a control showing the filter admits code). 1. The "no live-Redis path" parenthetical claimed both names appeared nowhere in the package. Re-scanned the WHOLE package rather than `src/`: `RUN_REAL_REDIS` and `skipIf` do occur nowhere outside that sentence, so the escape hatch genuinely never existed — but `REDIS_URL` occurs at `README.md:42`, as the env var a caller feeds to `createRedisClient()`. The sentence now says exactly that instead of over-reaching. 2. `multi` was listed among the commands carrying their v5 overloads verbatim into `RedisCommander.d.ts`. That is false for `multi` specifically: it is declared there in NEITHER version (measured 0 and 0, against a control member `exec` reading 1 and 1); it lives on `Transaction`. Measured on its real home, it DID change — all four overloads went from returning `ChainableCommander` to returning it parameterised by a reply mapping. It is removed from the verbatim list and given its own bullet stating why the change is inert here: the parameter defaults to "resp2", the class defaults to "legacy" and extends `Transaction` at "resp2", so a client built without `replyMapping` resolves `multi()` to the non-RESP3 instantiation. 3. "v6's one substantive change reachable from this package" was wrong in the other direction: v6 also changes connection defaults that are NOT opt-in — `protocol: 3` (no such option in v5) and `keepAlive` 0 to 30000. The sentence is scoped to "the surface these suites drive", which is what was measured, and the connection-default changes are named as changed-but-unexercised, since no suite here calls `createRedisClient()`. Two further wording fixes, each closing a way the note could read as true when it is not: - the expiry clause now also fires on a RESOLVED-version move under an unchanged caret range, which a lockfile bump alone will do; as written it expired only when a range moved - the unmet-peer warning is scoped to a resolving install, since a frozen re-link prints nothing Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --- .../src/redis.contract.test.ts | 54 +++++++++++++------ 1 file changed, 37 insertions(+), 17 deletions(-) diff --git a/packages/services/service-cluster-redis/src/redis.contract.test.ts b/packages/services/service-cluster-redis/src/redis.contract.test.ts index b015f34f3f..481614adc8 100644 --- a/packages/services/service-cluster-redis/src/redis.contract.test.ts +++ b/packages/services/service-cluster-redis/src/redis.contract.test.ts @@ -6,39 +6,59 @@ * There is no live-Redis path in this file: every suite below runs on the * mock. (An earlier version of this header promised `RUN_REAL_REDIS=1` + * `REDIS_URL` and "conditional `describe.skipIf` blocks at the bottom" — - * no such blocks were ever here, and the two names appeared nowhere in - * this package outside that sentence.) + * no such blocks were ever here. Scanning the whole package, not just + * `src/`: `RUN_REAL_REDIS` and `skipIf` occur nowhere outside that + * sentence, so the escape hatch never existed. `REDIS_URL` does occur, at + * `README.md:42` — but as the env var a caller feeds to + * `createRedisClient()`, which is unrelated to any test path.) * * ## The double is one major version behind the client it doubles * * This package depends on `ioredis@^6`, while `ioredis-mock@8.13.1` - * declares `peerDependencies: { ioredis: "^5" }` — so `pnpm install` - * prints an unmet-peer warning for it. That gap is real, and it is + * declares `peerDependencies: { ioredis: "^5" }` — so a resolving + * `pnpm install` prints an unmet-peer warning for it (a frozen re-link + * prints nothing). That gap is real, and it is * declared here rather than closed, because it was measured to be inert * on the surface these suites actually drive. Measured against ioredis * 5.11.1 (newest release satisfying the mock's `^5` peer) and 6.0.0 (the * version resolved in this workspace): * * - Every Redis command issued by this package's `src/*.ts` — get, set, - * del, incr, incrby, pttl, watch, unwatch, multi, exec, publish, - * subscribe, unsubscribe, quit, eval — carries all of its v5 - * overloads verbatim into v6's `RedisCommander.d.ts`. `set` is a - * strict superset there (v6 adds the IFEQ/IFNE/IFDEQ/IFDNE tokens); - * nothing used here was removed or re-shaped. - * - v6's one substantive change reachable from this package is RESP3 - * reply mapping, and it is opt-in: the class is declared with a - * `ReplyMapping` parameter defaulting to "legacy", `ChainableCommander` - * defaults to "resp2", and `duplicate()` with no override inherits the - * caller's mapping. This package never passes `replyMapping`, so every - * reply shape it sees is still the v5 one. + * del, incr, incrby, pttl, watch, unwatch, exec, publish, subscribe, + * unsubscribe, quit, eval — carries all of its v5 overloads verbatim + * into v6's `RedisCommander.d.ts`. `set` is a strict superset there + * (v6 adds the IFEQ/IFNE/IFDEQ/IFDNE tokens); nothing used here was + * removed or re-shaped. + * - `multi()` is deliberately not in that list: it is declared on + * `Transaction` (`transaction.d.ts`) and appears in + * `RedisCommander.d.ts` in neither version. Measured separately, it + * DID change — all four overloads went from returning + * `ChainableCommander` to `ChainableCommander` parameterised by a + * mapping. It is inert here because the parameter defaults to "resp2" + * and a client built without `replyMapping` reaches it as such: the + * class defaults to "legacy" and extends `Transaction` at "resp2". + * So this package's `multi()` resolves to the non-RESP3 instantiation, + * and `exec()`'s own declaration is byte-identical across the pair. + * - v6's one substantive change reachable from the surface these suites + * drive is RESP3 reply mapping, and it is opt-in: the class is + * declared with a `ReplyMapping` parameter defaulting to "legacy", + * `ChainableCommander` defaults to "resp2", and `duplicate()` with no + * override inherits the caller's mapping. This package never passes + * `replyMapping`, so every reply shape these suites see is the v5 one. + * ⚠️ Scoped deliberately: v6 also changes connection defaults that are + * NOT opt-in — `protocol: 3` (no such option in v5) and `keepAlive` + * 0 -> 30000. Those are reached in production through + * `createRedisClient()`, which no suite here calls (see below), so + * they are changed-but-unexercised rather than absent. * - The three `RedisOptions` keys client.ts sets — lazyConnect, * maxRetriesPerRequest, enableAutoPipelining — are declared * identically in both versions. * * That named set is the whole basis for the claim; it is not a statement * about ioredis 5 vs 6 in general. If the `ioredis` or `ioredis-mock` - * range in package.json moves, this paragraph expires and the diff has to - * be re-taken. + * range in package.json moves — OR if the version either one RESOLVES to + * moves under an unchanged caret range, which a lockfile bump alone will + * do — this paragraph expires and the diff has to be re-taken. * * ## What these suites therefore do NOT certify *