Use simplified key specs in cluster slot verification

[TalZaccai]

Problem:

• ClusterSlotVerificationInput currently stores pre-computed key search fields (firstKey, lastKey, step, keyNumOffset) populated by ClusterProvider.ExtractKeySpecs - this logic may ignore some scenarios such as multiple key specifications per command.
• ExtractKeySpecs duplicated key spec resolution logic that already existed in TryGetKeySearchArgsFromSimpleKeySpec, with its own implementation handling multiple key spec types (BeginSearchIndex, FindKeysRange, FindKeysKeyNum).
• The keyNumOffset field was a workaround for 2-spec commands where a count argument sits between keys — TryGetKeySearchArgsFromSimpleKeySpec handles this naturally by resolving each spec independently

Changes:

• ClusterSlotVerificationInput — Replaced firstKey, lastKey, step, keyNumOffset with SimpleRespKeySpec[] keySpecs and bool isSubCommand
• MultiKeySlotVerify — Now iterates over keySpecs, resolves each via TryGetKeySearchArgsFromSimpleKeySpec, and verifies keys per-spec; eliminates keyNumOffset skip hack
• CanServeSlot — Uses TryGetSimpleRespCommandInfo instead of TryFastGetRespCommandInfo + ExtractKeySpecs
• ClusterSession.ProcessClusterCommands — Same pattern change; removed unused commandInfo variable
• ClusterProvider.ExtractKeySpecs — Deleted entirely (65 lines)
• IClusterProvider — Removed ExtractKeySpecs from interface
• TryGetKeySearchArgsFromSimpleKeySpec — Changed from internal to public for cross-project access

Result:

• Net -73 lines (42 added, 115 removed)
• Single code path for key spec resolution across both transaction locking and slot verification

[Copilot]

Pull request overview

This PR refactors cluster slot verification to rely on the existing simplified key-spec resolution path (via SimpleRespKeySpec + TryGetKeySearchArgsFromSimpleKeySpec) instead of maintaining a separate “precomputed first/last/step” extraction implementation.

Changes:

• Replaces ClusterSlotVerificationInput’s precomputed index fields with SimpleRespKeySpec[] keySpecs plus an isSubCommand flag.
• Updates multi-key slot verification to iterate over all key specs and resolve key positions per-spec via TryGetKeySearchArgsFromSimpleKeySpec.
• Removes ExtractKeySpecs from IClusterProvider and deletes its implementation; updates call sites to use TryGetSimpleRespCommandInfo.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
libs/server/SessionParseStateExtensions.cs	Exposes TryGetKeySearchArgsFromSimpleKeySpec publicly for reuse across projects.
libs/server/Resp/RespServerSessionSlotVerify.cs	Switches slot verification setup to SimpleRespCommandInfo key specs and sets isSubCommand (incl. BITOP special-case).
libs/server/Cluster/IClusterProvider.cs	Removes ExtractKeySpecs from the provider interface.
libs/server/Cluster/ClusterSlotVerificationInput.cs	Replaces index/step fields with keySpecs + isSubCommand.
libs/cluster/Session/SlotVerification/ClusterSlotVerify.cs	Reworks multi-key slot verification to resolve/verify keys per key spec.
libs/cluster/Session/ClusterSession.cs	Uses TryGetSimpleRespCommandInfo and keySpecs for cluster subcommand slot verification.
libs/cluster/Server/ClusterProvider.cs	Deletes now-unused ExtractKeySpecs implementation.

Comments suppressed due to low confidence (1)

libs/server/SessionParseStateExtensions.cs:933

• TryGetKeySearchArgsFromSimpleKeySpec can read outside the SessionParseState buffer in Release builds. In the keyword-search branch it uses step = -1 when keySpec.BeginSearch.Index < 0, but the loop condition is i < parseState.Count, so i will eventually become negative and GetArgSliceByRef(i) will dereference before bufferPtr (e.g., MIGRATE has a BeginSearchKeyword spec with StartFrom: -2 in libs/resources/RespCommandsInfo.json). Also, key-num/range cases rely on Debug.Assert for bounds (keyNumIdx, lastKeyIdx), which doesn’t protect Release builds and can lead to OOB access via TryGetInt/GetArgSliceByRef.

Please add runtime bounds checks and return false for invalid/missing args: use a backward-scan condition (i >= 0) when scanning with step < 0, validate i + 1 < parseState.Count when a keyword is found, validate keyNumIdx is in-range and TryGetInt succeeds, and ensure computed firstIdx/lastIdx stay within [0, parseState.Count - 1].

        public static bool TryGetKeySearchArgsFromSimpleKeySpec(this ref SessionParseState parseState, SimpleRespKeySpec keySpec, bool isSubCommand, out (int firstIdx, int lastIdx, int step) searchArgs)
        {
            searchArgs = (-1, -1, -1);

            // Determine the starting index for searching keys

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.