Repository navigation
fix(mcp): several database tools report success for operations that did nothing or did something else #1215
Description
Summary
During a tool sweep on 2026-10-09 (Redis 8.6.1), these tools returned a success message that does not match what happened. Agents trust tool output literally, so each of these produces a confidently wrong next step.
| Tool | Call | Output | What actually happened |
|---|---|---|---|
redis_expire |
seconds: -5 (also 0) |
OK - TTL set to -5 seconds on 'k' |
Redis deletes the key immediately for TTL <= 0. Key is gone. |
redis_json_toggle |
path points at a number | Toggled 'k' at '$.n': [(nil)] |
Nothing toggled; nil means the path was not a boolean |
redis_json_numincrby |
path does not exist | New value at '$.missing': [] |
Nothing incremented; empty array means no path matched |
redis_zrangebyscore |
count: 1, no offset |
returned all members | count silently dropped because LIMIT needs both; description says "requires offset" but nothing enforces it |
redis_keys / redis_scan |
limit: 0 |
No keys found matching pattern |
Pattern had 50 matches; 0 was treated as "return none" |
redis_seed |
key_pattern without {i}, count: 5 |
Seed complete: 5 string items seeded |
Wrote the same key 5 times; 1 key exists |
redis_xrange |
start: "+", end: "-" |
(empty stream or key 'orders:stream' not found) |
Key exists with 40 entries; range was inverted |
redis_lrange |
start > stop |
(empty list or key not found) |
Key exists; range was inverted |
redis_info |
unknown section |
no output at all | Should say the section is unknown |
redis_getrange |
key missing / empty result | no output at all | Should print "" or (empty string) |
Proposed fixes
redis_expire: rejectseconds <= 0with a message pointing atredis_del/redis_persist, or say explicitly "TTL <= 0 deletes the key; 'k' has been deleted". Same check forredis_hexpire.- JSON tools returning
[nil]or[]for a$path: treat as "path matched nothing / wrong type" and say so, mirroring the per-path output ofjson_arrlen. redis_zrangebyscore: defaultoffsetto 0 whencountis set, or return an input error.redis_keys/redis_scan: treatlimit: 0as an input error (minimum 1) or document it as unlimited and implement that.redis_seed: warn whenkey_patternhas no{i}andcount > 1.redis_xrange/redis_lrange: when the key exists (cheapEXISTSor just drop the "or key not found" guess), say "no entries in range" instead.redis_infounknown section: non-empty message.redis_getrangeempty result: print an explicit empty marker.
Related
#1212, #1214 (nil-reply crashes). Same theme: tool output must match the Redis semantics so the agent can self-correct.
Activity
joshrotenberg commentedon Oct 13, 2026
Pagination-only checkpoint: draft #1235 at
e23c2ff6fbe0ae1e0e114f7cb88fc34169e7e34fnow rejects incompleteredis_zrangebyscoreoffset/count pairs rather than silently sending an unbounded query. Both omitted and complete pairs retain existing behavior. Two real-tool synthetic RESP2 regressions cover six invalid and four valid calls; the baseline false-success control discriminates the fix. Formatting, strict locked Clippy, eight catalog/policy tests and database/test-support-only compilation pass; independent source-only Due review approved with optional nits. Current-head hosted checks/human review remain required. Connection acquisition stays in the shared macro; the guarantee is no data command after incomplete input, not no connection. Leaves #1215 open for the remaining behaviors and acceptance; no merge/release or schema/credential/backend/default change.joshrotenberg commentedon Oct 13, 2026
Unmatched-JSONPath checkpoint: draft #1236 at
9f33d860b14d09fd548637f45ddfe348a3dd2af8now reportsNothing incremented: path '...' matched no valuesfor an empty RESP2 JSON-array reply, in line with the official JSON.NUMINCRBY return contract. Nonempty, legacy and error outcomes remain unchanged. Three actual-tool regressions / fifteen synthetic RESP2 cases pass; corrected baseline reproduces the misleadingNew value ... [], with preservation/error controls passing. Formatting, strict locked Clippy, eight catalog/policy tests and minimal-feature compilation pass; independent source-only Due review approved. Current-head CI/human review still required. Other JSON/non-numeric no-op cases and RESP3 behavior are not claimed fixed. Leaves #1215 open; no merge/release, schema, policy, credential/backend/default or audit-suppression changes.joshrotenberg commentedon Oct 13, 2026
Scoped follow-up: draft #1240 at
a308a9ce4e5aca2c220178d1e632cdeb863c9b45now reports empty LRANGE/XRANGE replies as no elements/entries returned for the requested bounds, without guessing key state or issuing an existence query.Original-code baseline reproduces both inverted-range message failures while unchanged controls pass. Candidate passes 13 actual-tool synthetic RESP2 cases and 178 MCP library tests; formatting, strict all-target/all-feature Clippy and minimal-feature test compilation pass. Independent source-only Due review found no blocker; caller verified the diff and results. Hosted exact-head checks and normal human review remain pending, so the PR stays draft.
This is output accuracy only; wire arguments/count/defaults, nonempty/error output, schema/policy/target/credential/backend contracts are unchanged. Synthetic replies do not prove live key existence. Leaves #1215 open for the remaining TTL, JSON, zero-limit, seed and acceptance work.
fix(mcp): refuse non-positive TTLs in redis_expire and redis_hexpire
fix(mcp): refuse non-positive TTLs in redis_expire and redis_hexpire (#…