Merge pull request #24399 from lambdalisue/fix-messagepack-rpc

feat(msgpack_rpc): add a new `msgpack-rpc` client type to fix behavior with MessagePack-RPC compliant clients

runtime/doc/api.txt

@@ -1336,7 +1336,11 @@ nvim_set_client_info({name}, {version}, {type}, {methods}, {attributes})
       • {type}        Must be one of the following values. Client libraries
                       should default to "remote" unless overridden by the
                       user.
-                      • "remote" remote client connected to Nvim.
+                      • "remote" remote client connected "Nvim flavored"
+                        MessagePack-RPC (responses must be in reverse order of
+                        requests). |msgpack-rpc|
+                      • "msgpack-rpc" remote client connected to Nvim via
+                        fully MessagePack-RPC compliant protocol.
                       • "ui" gui frontend
                       • "embedder" application using Nvim as a component (for
                         example, IDE/editor implementing a vim mode).

runtime/doc/news.txt

@@ -152,6 +152,9 @@ The following new APIs and features were added.
 • Functions that take a severity as an optional parameter (e.g.
   |vim.diagnostic.get()|) now also accept a list of severities |vim.diagnostic.severity|
 
+• New RPC client type `msgpack-rpc` is added for `nvim_set_client_info` to
+  support fully MessagePack-RPC compliant clients.
+
 ==============================================================================
 CHANGED FEATURES                                                 *news-changed*
 

runtime/lua/vim/_meta/api.lua

@@ -1679,7 +1679,11 @@ function vim.api.nvim_select_popupmenu_item(item, insert, finish, opts) end
 --- @param type string Must be one of the following values. Client libraries
 ---                   should default to "remote" unless overridden by the
 ---                   user.
----                   • "remote" remote client connected to Nvim.
+---                   • "remote" remote client connected "Nvim flavored"
+---                     MessagePack-RPC (responses must be in reverse order of
+---                     requests). `msgpack-rpc`
+---                   • "msgpack-rpc" remote client connected to Nvim via
+---                     fully MessagePack-RPC compliant protocol.
 ---                   • "ui" gui frontend
 ---                   • "embedder" application using Nvim as a component (for
 ---                     example, IDE/editor implementing a vim mode).

src/nvim/api/vim.c

@@ -1500,7 +1500,10 @@ Array nvim_get_api_info(uint64_t channel_id, Arena *arena)
 ///     - "commit" hash or similar identifier of commit
 /// @param type Must be one of the following values. Client libraries should
 ///             default to "remote" unless overridden by the user.
-///     - "remote" remote client connected to Nvim.
+///     - "remote" remote client connected "Nvim flavored" MessagePack-RPC (responses
+///                must be in reverse order of requests). |msgpack-rpc|
+///     - "msgpack-rpc" remote client connected to Nvim via fully MessagePack-RPC
+///                     compliant protocol.
 ///     - "ui" gui frontend
 ///     - "embedder" application using Nvim as a component (for example,
 ///                  IDE/editor implementing a vim mode).

src/nvim/eval/funcs.c

@@ -6730,7 +6730,7 @@ static void f_rpcrequest(typval_T *argvars, typval_T *rettv, EvalFuncData fptr)
     const char *name = NULL;
     Channel *chan = find_channel(chan_id);
     if (chan) {
-      name = rpc_client_name(chan);
+      name = get_client_info(chan, "name");
     }
     msg_ext_set_kind("rpc_error");
     if (name) {

src/nvim/msgpack_rpc/channel.c

@@ -306,6 +306,17 @@ end:
   channel_decref(channel);
 }
 
+static ChannelCallFrame *find_call_frame(RpcState *rpc, uint32_t request_id)
+{
+  for (size_t i = 0; i < kv_size(rpc->call_stack); i++) {
+    ChannelCallFrame *frame = kv_Z(rpc->call_stack, i);
+    if (frame->request_id == request_id) {
+      return frame;
+    }
+  }
+  return NULL;
+}
+
 static void parse_msgpack(Channel *channel)
 {
   Unpacker *p = channel->rpc.unpacker;
@@ -321,13 +332,15 @@ static void parse_msgpack(Channel *channel)
       }
       arena_mem_free(arena_finish(&p->arena));
     } else if (p->type == kMessageTypeResponse) {
-      ChannelCallFrame *frame = kv_last(channel->rpc.call_stack);
-      if (p->request_id != frame->request_id) {
+      ChannelCallFrame *frame = channel->rpc.client_type == kClientTypeMsgpackRpc
+        ? find_call_frame(&channel->rpc, p->request_id)
+        : kv_last(channel->rpc.call_stack);
+      if (frame == NULL || p->request_id != frame->request_id) {
         char buf[256];
         snprintf(buf, sizeof(buf),
-                 "ch %" PRIu64 " returned a response with an unknown request "
-                 "id. Ensure the client is properly synchronized",
-                 channel->id);
+                 "ch %" PRIu64 " (type=%" PRIu32 ") returned a response with an unknown request "
+                 "id %" PRIu32 ". Ensure the client is properly synchronized",
+                 channel->id, (unsigned)channel->rpc.client_type, p->request_id);
         chan_close_with_error(channel, buf, LOGLVL_ERR);
       }
       frame->returned = true;
@@ -691,6 +704,25 @@ void rpc_set_client_info(uint64_t id, Dictionary info)
 
   api_free_dictionary(chan->rpc.info);
   chan->rpc.info = info;
+
+  // Parse "type" on "info" and set "client_type"
+  const char *type = get_client_info(chan, "type");
+  if (type == NULL || strequal(type, "remote")) {
+    chan->rpc.client_type = kClientTypeRemote;
+  } else if (strequal(type, "msgpack-rpc")) {
+    chan->rpc.client_type = kClientTypeMsgpackRpc;
+  } else if (strequal(type, "ui")) {
+    chan->rpc.client_type = kClientTypeUi;
+  } else if (strequal(type, "embedder")) {
+    chan->rpc.client_type = kClientTypeEmbedder;
+  } else if (strequal(type, "host")) {
+    chan->rpc.client_type = kClientTypeHost;
+  } else if (strequal(type, "plugin")) {
+    chan->rpc.client_type = kClientTypePlugin;
+  } else {
+    chan->rpc.client_type = kClientTypeUnknown;
+  }
+
   channel_info_changed(chan, false);
 }
 
@@ -699,14 +731,15 @@ Dictionary rpc_client_info(Channel *chan)
   return copy_dictionary(chan->rpc.info, NULL);
 }
 
-const char *rpc_client_name(Channel *chan)
+const char *get_client_info(Channel *chan, const char *key)
+  FUNC_ATTR_NONNULL_ALL
 {
   if (!chan->is_rpc) {
     return NULL;
   }
   Dictionary info = chan->rpc.info;
   for (size_t i = 0; i < info.size; i++) {
-    if (strequal("name", info.items[i].key.data)
+    if (strequal(key, info.items[i].key.data)
         && info.items[i].value.type == kObjectTypeString) {
       return info.items[i].value.data.string.data;
     }

src/nvim/msgpack_rpc/channel_defs.h

@@ -14,6 +14,16 @@
 typedef struct Channel Channel;
 typedef struct Unpacker Unpacker;
 
+typedef enum {
+  kClientTypeUnknown = -1,
+  kClientTypeRemote = 0,
+  kClientTypeMsgpackRpc = 5,
+  kClientTypeUi = 1,
+  kClientTypeEmbedder = 2,
+  kClientTypeHost = 3,
+  kClientTypePlugin = 4,
+} ClientType;
+
 typedef struct {
   uint32_t request_id;
   bool returned, errored;
@@ -37,6 +47,7 @@ typedef struct {
   uint32_t next_request_id;
   kvec_t(ChannelCallFrame *) call_stack;
   Dictionary info;
+  ClientType client_type;
 } RpcState;
 
 #endif  // NVIM_MSGPACK_RPC_CHANNEL_DEFS_H