diff --git a/tests/test-chat.cpp b/tests/test-chat.cpp index 4378a8db71..e12805e2f3 100644 --- a/tests/test-chat.cpp +++ b/tests/test-chat.cpp @@ -69,8 +69,8 @@ static common_chat_msg normalize(const common_chat_msg & msg) { for (auto & tool_call : normalized.tool_calls) { try { tool_call.arguments = json::parse(tool_call.arguments).dump(); - } catch (const std::exception &) { - // Do nothing + } catch (const std::exception &e) { + LOG_DBG("Normalize failed on tool call: %s\n", e.what()); } } return normalized; @@ -183,7 +183,7 @@ static void assert_msg_equals(const common_chat_msg & expected, const common_cha } } -common_chat_tool special_function_tool { +static common_chat_tool special_function_tool { /* .name = */ "special_function", /* .description = */ "I'm special", /* .parameters = */ R"({ @@ -197,7 +197,7 @@ common_chat_tool special_function_tool { "required": ["arg1"] })", }; -common_chat_tool special_function_tool_with_optional_param { +static common_chat_tool special_function_tool_with_optional_param { /* .name = */ "special_function_with_opt", /* .description = */ "I'm special but have optional stuff", /* .parameters = */ R"({ diff --git a/tools/cli/cli.cpp b/tools/cli/cli.cpp index f9ad5094b6..1e63ab9aa5 100644 --- a/tools/cli/cli.cpp +++ b/tools/cli/cli.cpp @@ -22,7 +22,7 @@ # include #endif -const char * LLAMA_ASCII_LOGO = R"( +static const char * LLAMA_ASCII_LOGO = R"( ▄▄ ▄▄ ██ ██ ██ ██ ▀▀█▄ ███▄███▄ ▀▀█▄ ▄████ ████▄ ████▄ @@ -39,7 +39,7 @@ static bool should_stop() { } #if defined(__unix__) || (defined(__APPLE__) && defined(__MACH__)) || defined(_WIN32) -static void signal_handler(int) { +static void signal_handler(int /*unused*/) { if (g_is_interrupted.load()) { // second Ctrl+C - exit immediately // make sure to clear colors before exiting (not using LOG or console.cpp here to avoid deadlock) @@ -119,9 +119,9 @@ struct cli_context { out_result = std::move(result); return curr_content; } - auto res_partial = dynamic_cast(result.get()); + auto *res_partial = dynamic_cast(result.get()); if (res_partial) { - out_timings = std::move(res_partial->timings); + out_timings = res_partial->timings; for (const auto & diff : res_partial->oaicompat_msg_diffs) { if (!diff.content_delta.empty()) { if (is_thinking) { @@ -144,9 +144,9 @@ struct cli_context { } } } - auto res_final = dynamic_cast(result.get()); + auto *res_final = dynamic_cast(result.get()); if (res_final) { - out_timings = std::move(res_final->timings); + out_timings = res_final->timings; out_result = std::move(result); break; } @@ -168,10 +168,8 @@ struct cli_context { buf.assign((std::istreambuf_iterator(file)), std::istreambuf_iterator()); input_files.push_back(std::move(buf)); return mtmd_default_marker(); - } else { - std::string content((std::istreambuf_iterator(file)), std::istreambuf_iterator()); - return content; - } + } + return std::string((std::istreambuf_iterator(file)), std::istreambuf_iterator()); } common_chat_params format_chat() const { @@ -365,7 +363,8 @@ int main(int argc, char ** argv) { // process commands if (string_starts_with(buffer, "/exit")) { break; - } else if (string_starts_with(buffer, "/regen")) { + } + if (string_starts_with(buffer, "/regen")) { if (ctx_cli.messages.size() >= 2) { size_t last_idx = ctx_cli.messages.size() - 1; ctx_cli.messages.erase(last_idx); @@ -418,7 +417,7 @@ int main(int argc, char ** argv) { while (true) { server_task_result_ptr result; std::string assistant_content = ctx_cli.generate_completion(timings, result); - auto res_final = dynamic_cast(result.get()); + auto * res_final = dynamic_cast(result.get()); if (res_final && !res_final->oaicompat_msg.tool_calls.empty()) { ctx_cli.messages.push_back(res_final->oaicompat_msg.to_json_oaicompat()); @@ -443,38 +442,36 @@ int main(int argc, char ** argv) { } std::string request_id = tc.name + tc.arguments; - if (!mcp.get_yolo() && ctx_cli.approved_requests.find(request_id) == ctx_cli.approved_requests.end()) { - // Prompt user - fprintf(stdout, "\n\033[1;33mTool call: %s\033[0m\n", tc.name.c_str()); - fprintf(stdout, "Arguments: %s\n", args.dump(2).c_str()); - fprintf(stdout, "Approve? [y]es, [n]o, [A]lways allow feature: "); - fflush(stdout); + if (!mcp.get_yolo() && + ctx_cli.approved_requests.find(request_id) == ctx_cli.approved_requests.end()) { + // Prompt user + fprintf(stdout, "\n\n\033[1;33mTool call: %s\033[0m\n", tc.name.c_str()); + fprintf(stdout, "Arguments: %s\n", args.dump(2).c_str()); + fprintf(stdout, "Approve? [y]es, [n]o, [A]lways allow feature: "); + fflush(stdout); - char c = ' '; - std::string line; - // We are in main loop which might have console settings. - // Use console::readline or similar? - // But for single char input, standard cin/getline might interfere with console lib state if it's separate. - // Given `console::readline` is used above, let's use standard input as fallback or just try cin. - - // Simple blocking read - std::cin >> c; - std::getline(std::cin, line); // consume rest + char c = ' '; + std::string line; - if (c == 'y' || c == 'Y') { - // approved once - } else if (c == 'A') { - ctx_cli.approved_requests.insert(request_id); - } else { - json err_msg = { - { "role", "tool" }, + console::readline(line, false); + if (!line.empty()) { + c = line[0]; + } + + if (c == 'y' || c == 'Y') { + // approved once + } else if (c == 'A') { + ctx_cli.approved_requests.insert(request_id); + } else { + json err_msg = { + { "role", "tool" }, { "content", "User denied tool execution" }, - { "tool_call_id", tc.id }, - { "name", tc.name } + { "tool_call_id", tc.id }, + { "name", tc.name } }; ctx_cli.messages.push_back(err_msg); continue; - } + } } json res = mcp.call_tool(tc.name, args); diff --git a/tools/cli/mcp.hpp b/tools/cli/mcp.hpp index 62662f1ef0..422cd2a807 100644 --- a/tools/cli/mcp.hpp +++ b/tools/cli/mcp.hpp @@ -188,6 +188,11 @@ class mcp_server { } json send_request(const std::string & method, const json & params = json::object()) { + if (!running) { + LOG_ERR("Cannot send request to %s: server not running\n", name.c_str()); + return nullptr; + } + int id; std::future future; { @@ -206,13 +211,19 @@ class mcp_server { std::string req_str = req.dump() + "\n"; FILE * stdin_file = subprocess_stdin(&process); if (stdin_file) { + LOG_DBG("MCP request to %s [id=%d]: %s\n", name.c_str(), id, method.c_str()); fwrite(req_str.c_str(), 1, req_str.size(), stdin_file); fflush(stdin_file); + } else { + LOG_ERR("Cannot send request to %s: stdin is null\n", name.c_str()); + std::lock_guard lock(mutex); + pending_requests.erase(id); + return nullptr; } // Wait for response with timeout - if (future.wait_for(std::chrono::seconds(10)) == std::future_status::timeout) { - LOG_ERR("Timeout waiting for response from %s (method: %s)\n", name.c_str(), method.c_str()); + if (future.wait_for(std::chrono::seconds(30)) == std::future_status::timeout) { + LOG_ERR("Timeout waiting for response from %s (method: %s, id: %d)\n", name.c_str(), method.c_str(), id); std::lock_guard lock(mutex); pending_requests.erase(id); return nullptr; @@ -310,6 +321,7 @@ class mcp_server { private: void read_loop() { + LOG_DBG("MCP read_loop started for %s\n", name.c_str()); char buffer[4096]; while (!stop_read && running) { unsigned bytes_read = subprocess_read_stdout(&process, buffer, sizeof(buffer)); @@ -321,7 +333,7 @@ class mcp_server { // when stop() is called concurrently. // Just break the loop. The process is likely dead or dying. if (!stop_read) { - LOG_ERR("MCP process died (stdout closed)\n"); + LOG_ERR("MCP server %s: read_loop exiting (stdout closed/EOF)\n", name.c_str()); } running = false; break; @@ -339,28 +351,36 @@ class mcp_server { try { json msg = json::parse(line); - if (msg.contains("id")) { - // Response - int id = msg["id"].get(); + if (msg.contains("id") && !msg["id"].is_null()) { + // Response - handle both int and string IDs (JSON-RPC allows both) + int id; + if (msg["id"].is_string()) { + id = std::stoi(msg["id"].get()); + } else { + id = msg["id"].get(); + } std::lock_guard lock(mutex); if (pending_requests.count(id)) { + LOG_DBG("MCP response received from %s [id=%d]\n", name.c_str(), id); pending_requests[id].set_value(msg); pending_requests.erase(id); } else { - // ID not found + LOG_WRN("MCP response for unknown id %d from %s: %s\n", id, name.c_str(), line.c_str()); } } else { // Notification or request from server -> ignore for now or log // MCP servers might send notifications (e.g. logging) - LOG_ERR("MCP Notification from %s: %s\n", name.c_str(), line.c_str()); + LOG_INF("MCP Notification from %s: %s\n", name.c_str(), line.c_str()); } } catch (const std::exception & e) { // Not a full JSON yet? Or invalid? // If it was a line, it should be valid JSON-RPC - LOG_WRN("Failed to parse JSON from %s: %s\n", name.c_str(), e.what()); + LOG_WRN("Failed to parse JSON from %s: %s (line: %s)\n", name.c_str(), e.what(), line.c_str()); } } } + LOG_DBG("MCP read_loop exiting for %s (stop_read=%d, running=%d)\n", + name.c_str(), stop_read.load(), running.load()); } void err_loop() {