From e6d6cc4e879cce51f3e462849ee1dc601001c1f9 Mon Sep 17 00:00:00 2001 From: niukanen1 <57656076+niukanen1@users.noreply.github.com> Date: Mon, 28 Sep 2026 08:58:56 +0400 Subject: [PATCH 1/2] fix(db): stop asking for confirmation before Redis blocking commands the provider refuses The blocking list-pop commands are refused by query() before they reach the server, so the dialog asked for confirmation and the results panel then said the command will not run. The non-blocking forms stay in the vocabulary and the dialog asks for them as before --- docs/providers/redis.md | 5 ++++- src/lib/db/destructive-commands.ts | 17 ++++++++--------- tests/unit/db/destructive-commands.test.ts | 16 ++++++++++++++++ 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/docs/providers/redis.md b/docs/providers/redis.md index bcea89ec6..532152fcb 100644 --- a/docs/providers/redis.md +++ b/docs/providers/redis.md @@ -280,7 +280,10 @@ on a leading `{` as §3.4 does, and asks before running any command in its Redis vocabulary - key, expiry, string, hash, list, set, sorted-set and stream writes, the scripting entry points, and the server and access commands, with container commands such as `CONFIG SET` matched on their two-token spelling - while a body it cannot read (broken JSON, a JSON body whose -`command` is not a string) asks rather than staying silent. +`command` is not a string) asks rather than staying silent. The vocabulary names only commands the +provider runs: the blocking list-pop family is refused before it reaches the server +([§5.2b](#52b-commands-that-would-change-the-shared-connection-1107)), so their non-blocking forms are the ones the +gate asks about. ### 3.5 Reply normalisation into the shared grid diff --git a/src/lib/db/destructive-commands.ts b/src/lib/db/destructive-commands.ts index a168ea23f..953505e4c 100644 --- a/src/lib/db/destructive-commands.ts +++ b/src/lib/db/destructive-commands.ts @@ -72,7 +72,14 @@ const MONGODB_DESTRUCTIVE_OPERATIONS: ReadonlySet = new Set([ * `CLUSTER SHARDS` are the reads); every form of it reassigns a slot, which is what * puts it beside `CLUSTER RESET` and `CLUSTER FORGET`. * - * Every name here can reach the server: `runCommand` calls + * Every name here is a command the provider runs: `query()` refuses some commands + * before they reach the server (`sharedConnectionRefusal` in + * `src/lib/db/providers/keyvalue/redis.ts`, documented in section 5.2b of + * `docs/providers/redis.md`), and a confirmation followed by that refusal would be + * the double take this gate exists to avoid. The blocking list-pop commands + * (`BLPOP` and its `B...` family) are refused that way, so only their + * non-blocking forms (`LPOP`, `RPOP`, `LMPOP`, `LMOVE`, `RPOPLPUSH`, `ZPOPMIN`, + * `ZPOPMAX`, `ZMPOP`) are here. `runCommand` itself calls * `client.call(command, ...args)` with no allow-list of any kind, so the vocabulary * is bounded by what Redis accepts rather than by what this provider implements. * Container commands are spelled with their subcommand (`CONFIG SET`), which the @@ -133,16 +140,11 @@ const REDIS_DESTRUCTIVE_COMMANDS: ReadonlySet = new Set([ "LPOP", "RPOP", "LMPOP", - "BLPOP", - "BRPOP", - "BLMPOP", "LSET", "LREM", "LTRIM", "LMOVE", - "BLMOVE", "RPOPLPUSH", - "BRPOPLPUSH", // Sets "SPOP", "SREM", @@ -157,10 +159,7 @@ const REDIS_DESTRUCTIVE_COMMANDS: ReadonlySet = new Set([ "ZREMRANGEBYLEX", "ZPOPMIN", "ZPOPMAX", - "BZPOPMIN", - "BZPOPMAX", "ZMPOP", - "BZMPOP", "ZUNIONSTORE", "ZINTERSTORE", "ZDIFFSTORE", diff --git a/tests/unit/db/destructive-commands.test.ts b/tests/unit/db/destructive-commands.test.ts index 0d6ea0c38..234966c0b 100644 --- a/tests/unit/db/destructive-commands.test.ts +++ b/tests/unit/db/destructive-commands.test.ts @@ -126,6 +126,22 @@ describe("isDestructiveNonSqlQuery", () => { expect(isDestructiveNonSqlQuery(query, "redis")).toBe(true); }); + test.each<[string]>([ + ["BLPOP queue 0"], + ["BRPOP queue 0"], + ["BLMPOP 2 2 queue LEFT COUNT 1"], + ["BLMOVE src dst LEFT RIGHT 0"], + ["BRPOPLPUSH src dst 0"], + ["BZPOPMIN z 0"], + ["BZPOPMAX z 0"], + ["BZMPOP 2 1 z MIN COUNT 1"], + ])("does not ask before the Redis command %s, which the provider refuses before it reaches the server", (query) => { + // Since #1121 `RedisProvider.query()` refuses the blocking commands through + // `sharedConnectionRefusal`, so a confirmation followed by a refusal is the + // double take this gate exists to avoid. The dialog is for commands that run. + expect(isDestructiveNonSqlQuery(query, "redis")).toBe(false); + }); + test.each<[string]>([ ["GET k"], ["HGETALL user:1"], From 8956f33237e04ad36513f883329af7e10d917adf Mon Sep 17 00:00:00 2001 From: cevheri Date: Mon, 28 Sep 2026 15:03:25 +0300 Subject: [PATCH 2/2] test(db): pin the non-blocking pops the Redis gate still asks about (#1164) Only LPOP was asserted, so deleting any of the other seven from the vocabulary left every test green. Also names the refused family as list and sorted-set pops, since BZPOPMIN, BZPOPMAX and BZMPOP are not list commands. --- docs/providers/redis.md | 6 +++--- src/lib/db/destructive-commands.ts | 4 ++-- tests/unit/db/destructive-commands.test.ts | 14 ++++++++++++++ 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/docs/providers/redis.md b/docs/providers/redis.md index 532152fcb..eb074bb32 100644 --- a/docs/providers/redis.md +++ b/docs/providers/redis.md @@ -281,9 +281,9 @@ vocabulary - key, expiry, string, hash, list, set, sorted-set and stream writes, entry points, and the server and access commands, with container commands such as `CONFIG SET` matched on their two-token spelling - while a body it cannot read (broken JSON, a JSON body whose `command` is not a string) asks rather than staying silent. The vocabulary names only commands the -provider runs: the blocking list-pop family is refused before it reaches the server -([§5.2b](#52b-commands-that-would-change-the-shared-connection-1107)), so their non-blocking forms are the ones the -gate asks about. +provider runs: the blocking list and sorted-set pops are refused before they reach the server +([§5.2b](#52b-commands-that-would-change-the-shared-connection-1107)), so their non-blocking +forms are the ones the gate asks about. ### 3.5 Reply normalisation into the shared grid diff --git a/src/lib/db/destructive-commands.ts b/src/lib/db/destructive-commands.ts index 953505e4c..7c9d8ff22 100644 --- a/src/lib/db/destructive-commands.ts +++ b/src/lib/db/destructive-commands.ts @@ -76,8 +76,8 @@ const MONGODB_DESTRUCTIVE_OPERATIONS: ReadonlySet = new Set([ * before they reach the server (`sharedConnectionRefusal` in * `src/lib/db/providers/keyvalue/redis.ts`, documented in section 5.2b of * `docs/providers/redis.md`), and a confirmation followed by that refusal would be - * the double take this gate exists to avoid. The blocking list-pop commands - * (`BLPOP` and its `B...` family) are refused that way, so only their + * the double take this gate exists to avoid. The blocking list and sorted-set pops + * (`BLPOP`, `BZPOPMIN` and the rest of the `B` forms) are refused that way, so only their * non-blocking forms (`LPOP`, `RPOP`, `LMPOP`, `LMOVE`, `RPOPLPUSH`, `ZPOPMIN`, * `ZPOPMAX`, `ZMPOP`) are here. `runCommand` itself calls * `client.call(command, ...args)` with no allow-list of any kind, so the vocabulary diff --git a/tests/unit/db/destructive-commands.test.ts b/tests/unit/db/destructive-commands.test.ts index 234966c0b..68e3c3f50 100644 --- a/tests/unit/db/destructive-commands.test.ts +++ b/tests/unit/db/destructive-commands.test.ts @@ -142,6 +142,20 @@ describe("isDestructiveNonSqlQuery", () => { expect(isDestructiveNonSqlQuery(query, "redis")).toBe(false); }); + test.each<[string]>([ + ["LPOP queue"], + ["RPOP queue"], + ["LMPOP 1 queue LEFT COUNT 1"], + ["LMOVE src dst LEFT RIGHT"], + ["RPOPLPUSH src dst"], + ["ZPOPMIN z"], + ["ZPOPMAX z"], + ["ZMPOP 1 z MIN COUNT 1"], + ])("still asks before %s, the non-blocking form of a refused command", (query) => { + // The provider runs these, and each one removes what it returns. + expect(isDestructiveNonSqlQuery(query, "redis")).toBe(true); + }); + test.each<[string]>([ ["GET k"], ["HGETALL user:1"],