From 14f76ab1db86cca6e2f944b95b535f6a252559f8 Mon Sep 17 00:00:00 2001 From: "jabber.developer2" Date: Mon, 6 Jul 2026 16:28:36 +0300 Subject: [PATCH 1/4] feat(ai): support both responses and chat completions APIs Drive both OpenAI-compatible flavours from one code path. Requests default to /v1/responses and fall back once to /v1/chat/completions when the provider reports the endpoint missing (404/405/501) or rejects the payload shape (400/422); the working flavour is cached for the rest of the run as a first-attempt hint, with the other flavour kept as a fallback so a backend change self-corrects in-request. A per-provider override is available via /ai set api-type responses|chat-completions|auto and persisted as api_type= in the provider section. One payload builder serves both flavours (messages/stream vs input/stream/store); history and custom settings are serialized once per request and the per-flavour envelope is assembled per attempt under a single lock acquisition, so the fallback retry cannot double-count the prompt. ai_parse_response_typed() dispatches on the request flavour and splits extraction per envelope: chat completions anchored on choices[].message.content, responses on the output_text part with a string-aware backward scan bounded to that part's own object so a reasoning summary or a truncated body is never returned as the reply. Endpoint detection classifies wrong-model errors from the structured error code/type/message when present, so route-missing 404s from gateways no longer suppress the fallback; it preserves and surfaces the first payload-rejection error when the fallback also fails, and re-probes on an unparseable 2xx. resolved_api_type is written only through a locked helper guarded by a URL/api-type epoch, and api_url is snapshotted under settings_lock in the request and models-fetch threads, closing use-after-free and stale-cache races against /ai set provider. The models-fetch provider ref is taken on the main thread and released on every worker exit path. Reserved custom-setting keys are extended with input (store stays writable as a legitimate chat-completions parameter). --- src/ai/ai_client.c | 582 +++++++++++++++++++++++++------ src/ai/ai_client.h | 71 +++- src/command/cmd_ac.c | 25 ++ src/command/cmd_defs.c | 7 +- src/command/cmd_funcs.c | 30 +- src/config/preferences.c | 24 ++ src/config/preferences.h | 5 + tests/unittests/test_ai_client.c | 209 ++++++++++- tests/unittests/test_ai_client.h | 14 +- tests/unittests/unittests.c | 14 +- 10 files changed, 858 insertions(+), 123 deletions(-) diff --git a/src/ai/ai_client.c b/src/ai/ai_client.c index 4ca329d9..8bd48767 100644 --- a/src/ai/ai_client.c +++ b/src/ai/ai_client.c @@ -186,11 +186,13 @@ ai_json_escape(const gchar* str) return g_string_free(result, FALSE); } -/* Keys emitted as fixed payload fields; custom settings must not duplicate them */ +/* Keys emitted as fixed payload fields (in either API flavour); custom + * settings must not duplicate them */ static gboolean _is_reserved_payload_key(const gchar* key) { - return g_strcmp0(key, "model") == 0 || g_strcmp0(key, "messages") == 0 || g_strcmp0(key, "stream") == 0; + return g_strcmp0(key, "model") == 0 || g_strcmp0(key, "messages") == 0 + || g_strcmp0(key, "input") == 0 || g_strcmp0(key, "stream") == 0; } /* RFC 8259 scalars (number/true/false/null) may be emitted bare in a JSON payload */ @@ -199,7 +201,16 @@ _is_json_bare_token(const gchar* value) { if (g_strcmp0(value, "true") == 0 || g_strcmp0(value, "false") == 0 || g_strcmp0(value, "null") == 0) return TRUE; - return g_regex_match_simple("^-?(0|[1-9][0-9]*)(\\.[0-9]+)?([eE][+-]?[0-9]+)?$", value, 0, 0); + + /* Compile the number grammar once; this runs per custom setting per request */ + static gsize once = 0; + static GRegex* number_re = NULL; + if (g_once_init_enter(&once)) { + number_re = g_regex_new("^-?(0|[1-9][0-9]*)(\\.[0-9]+)?([eE][+-]?[0-9]+)?$", + G_REGEX_OPTIMIZE, 0, NULL); + g_once_init_leave(&once, 1); + } + return number_re && g_regex_match(number_re, value, 0, NULL); } /** @@ -392,6 +403,135 @@ _extract_json_string(const gchar* json, const gchar* field) return NULL; } +/* ======================================================================== + * API type helpers + * ======================================================================== */ + +gboolean +ai_api_type_from_string(const gchar* str, AIApiType* type) +{ + if (!str || !type) + return FALSE; + + if (g_strcmp0(str, "auto") == 0) { + *type = AI_API_TYPE_AUTO; + } else if (g_strcmp0(str, "responses") == 0) { + *type = AI_API_TYPE_RESPONSES; + } else if (g_strcmp0(str, "chat-completions") == 0) { + *type = AI_API_TYPE_CHAT_COMPLETIONS; + } else { + return FALSE; + } + return TRUE; +} + +const gchar* +ai_api_type_to_string(AIApiType type) +{ + switch (type) { + case AI_API_TYPE_RESPONSES: + return "responses"; + case AI_API_TYPE_CHAT_COMPLETIONS: + return "chat-completions"; + default: + return "auto"; + } +} + +static const gchar* +_api_type_endpoint(AIApiType type) +{ + return type == AI_API_TYPE_CHAT_COMPLETIONS ? "v1/chat/completions" : "v1/responses"; +} + +static AIApiType +_other_flavour(AIApiType type) +{ + return type == AI_API_TYPE_CHAT_COMPLETIONS ? AI_API_TYPE_RESPONSES : AI_API_TYPE_CHAT_COMPLETIONS; +} + +static gboolean +_names_model(const gchar* s) +{ + if (!s) + return FALSE; + /* "does not exist" alone also appears in route-missing bodies; require it + * to talk about a model */ + return strstr(s, "model_not_found") || strstr(s, "invalid_model") + || strstr(s, "unknown_model") || strstr(s, "model not found") + || (strstr(s, "model") && strstr(s, "does not exist")); +} + +/* A missing endpoint answers 404/405/501. A wrong model can also 404/405, but + * the error names the model (OpenAI code model_not_found, vLLM message "The + * model `x` does not exist"), so treat the endpoint as present then. Prefer + * the structured error fields; fall back to the raw body for servers that + * send no JSON error envelope. */ +static gboolean +_body_names_model_error(const gchar* body) +{ + if (!body) + return FALSE; + + const gchar* err = strstr(body, "\"error\""); + if (err) { + auto_gchar gchar* code = _extract_json_string(err, "code"); + auto_gchar gchar* type = _extract_json_string(err, "type"); + auto_gchar gchar* message = _extract_json_string(err, "message"); + if (code || type || message) + return _names_model(code) || _names_model(type) || _names_model(message); + } + return _names_model(body); +} + +static gboolean +_is_endpoint_missing(long http_code, const gchar* body) +{ + if (http_code != 404 && http_code != 405 && http_code != 501) + return FALSE; + return !_body_names_model_error(body); +} + +/* During an AUTO probe, move on to the other flavour when this endpoint is + * missing or rejects the payload shape (400/422) — but not on auth/rate-limit + * or model errors, which the other endpoint would fail the same way. */ +static gboolean +_should_try_other_flavour(long http_code, const gchar* body) +{ + if (_is_endpoint_missing(http_code, body)) + return TRUE; + if (http_code != 400 && http_code != 422) + return FALSE; + return !_body_names_model_error(body); +} + +/* Copy the provider URL under settings_lock, slash-terminated; @epoch_out + * (optional) captures api_epoch in the same critical section so a flavour + * stamped later can be tied to the URL it was probed against */ +static gchar* +_provider_base_url(AIProvider* provider, gint* epoch_out) +{ + pthread_mutex_lock(&provider->settings_lock); + gchar* base = g_strdup_printf("%s%s", provider->api_url, + g_str_has_suffix(provider->api_url, "/") ? "" : "/"); + if (epoch_out) + *epoch_out = g_atomic_int_get(&provider->api_epoch); + pthread_mutex_unlock(&provider->settings_lock); + return base; +} + +/* Single writer for resolved_api_type: takes settings_lock and, when + * @expected_epoch >= 0, drops the write if the epoch moved (the URL or + * api-type changed while the request was in flight) */ +static void +_provider_set_resolved(AIProvider* provider, AIApiType type, gint expected_epoch) +{ + pthread_mutex_lock(&provider->settings_lock); + if (expected_epoch < 0 || g_atomic_int_get(&provider->api_epoch) == expected_epoch) + g_atomic_int_set(&provider->resolved_api_type, type); + pthread_mutex_unlock(&provider->settings_lock); +} + /* ======================================================================== * Provider Management * ======================================================================== */ @@ -404,6 +544,9 @@ ai_provider_new(const gchar* name, const gchar* api_url) provider->api_url = g_strdup(api_url ? api_url : ""); provider->project_id = NULL; provider->default_model = NULL; + provider->api_type = AI_API_TYPE_AUTO; + provider->resolved_api_type = AI_API_TYPE_AUTO; + provider->api_epoch = 0; provider->settings = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, g_free); provider->models = NULL; provider->models_fresh = FALSE; @@ -519,6 +662,12 @@ ai_client_init(void) provider->default_model = g_strdup(dm); } + auto_gchar gchar* at = prefs_ai_get_api_type(provider->name); + AIApiType api_type; + if (at && ai_api_type_from_string(at, &api_type)) { + provider->api_type = api_type; + } + GList* settings = prefs_ai_list_settings(provider->name); for (GList* s = settings; s; s = g_list_next(s)) { const gchar* name = (const gchar*)s->data; @@ -573,9 +722,15 @@ _ai_add_provider_nopersist(const gchar* name, const gchar* api_url) /* Check if provider already exists */ AIProvider* existing = g_hash_table_lookup(providers, name); if (existing) { + pthread_mutex_lock(&existing->settings_lock); g_free(existing->api_url); existing->api_url = g_strdup(api_url); - return ai_provider_ref(existing); + g_atomic_int_inc(&existing->api_epoch); + /* New URL may serve a different API flavour; reset under the lock so a + * request thread cannot re-stamp the stale flavour for the new URL */ + g_atomic_int_set(&existing->resolved_api_type, AI_API_TYPE_AUTO); + pthread_mutex_unlock(&existing->settings_lock); + return existing; /* borrowed, same as the create path below */ } /* Create new provider (ref_count=1 owned by hash table) */ @@ -598,25 +753,18 @@ ai_add_provider(const gchar* name, const gchar* api_url) ai_client_init(); } - /* Check if provider already exists */ - AIProvider* existing = g_hash_table_lookup(providers, name); - if (existing) { - /* Update existing provider */ - g_free(existing->api_url); - existing->api_url = g_strdup(api_url); - /* Persist the updated URL to config */ - prefs_ai_set_provider(name, api_url); - log_info("Updated provider: %s", name); - return ai_provider_ref(existing); - } + gboolean existed = g_hash_table_lookup(providers, name) != NULL; - /* Persist the new provider to config */ + /* Persist first, then reuse the internal helper for both the update and + * create paths (it handles the locked URL swap or the fresh insert) */ prefs_ai_set_provider(name, api_url); - - /* Add to hash table (reuse internal helper) */ AIProvider* provider = _ai_add_provider_nopersist(name, api_url); - log_info("Added provider: %s (URL: %s)", name, api_url); + if (existed) { + log_info("Updated provider: %s", name); + } else { + log_info("Added provider: %s (URL: %s)", name, api_url); + } return provider; /* Caller gets non-owning pointer; hash table owns ref */ } @@ -747,6 +895,45 @@ ai_get_provider_default_model(const gchar* provider_name) return provider->default_model; } +gboolean +ai_set_provider_api_type(const gchar* provider_name, const gchar* api_type) +{ + AIApiType type; + if (!provider_name || !ai_api_type_from_string(api_type, &type)) + return FALSE; + + AIProvider* provider = ai_get_provider(provider_name); + if (!provider) { + log_warning("Provider '%s' not found for API type setting", provider_name); + return FALSE; + } + + /* Same protocol as a URL swap: bump the epoch under the lock so an + * in-flight request cannot re-stamp resolved_api_type after this reset */ + pthread_mutex_lock(&provider->settings_lock); + g_atomic_int_set(&provider->api_type, type); + g_atomic_int_inc(&provider->api_epoch); + g_atomic_int_set(&provider->resolved_api_type, AI_API_TYPE_AUTO); + pthread_mutex_unlock(&provider->settings_lock); + + if (type == AI_API_TYPE_AUTO) { + prefs_ai_remove_api_type(provider_name); + } else { + prefs_ai_set_api_type(provider_name, api_type); + } + log_info("API type for provider '%s' set to: %s", provider_name, ai_api_type_to_string(type)); + return TRUE; +} + +AIApiType +ai_get_provider_api_type(const gchar* provider_name) +{ + AIProvider* provider = provider_name ? ai_get_provider(provider_name) : NULL; + if (!provider) + return AI_API_TYPE_AUTO; + return g_atomic_int_get(&provider->api_type); +} + gboolean ai_set_provider_setting(const gchar* provider_name, const gchar* setting, const gchar* value) { @@ -812,6 +999,7 @@ typedef void (*ai_response_handler_t)(AIProvider* provider, const gchar* respons typedef struct { gchar* provider_name; + AIProvider* provider; /* owned ref taken on the main thread, or NULL */ gpointer user_data; gchar* request_url; ai_response_handler_t handler; @@ -874,22 +1062,27 @@ static gpointer _ai_generic_request_thread(gpointer data) { ai_request_t* req = (ai_request_t*)data; - AIProvider* provider = ai_get_provider(req->provider_name); - if (!provider) { - log_error("Provider '%s' not found for request", req->provider_name); - g_free(req->provider_name); - g_free(req->request_url); - g_free(req); - return NULL; - } - /* Keep the provider alive for the duration of this request. Without this, /ai remove provider X could free the provider - * struct (via the hash table's destroy-notify) while curl_easy_perform is still using it — classic UAF. */ - ai_provider_ref(provider); + /* Keep the provider alive for the duration of this request. Prefer the ref + * taken on the main thread (guaranteed pre-removal); the lookup fallback + * still has a small window against /ai remove provider. */ + AIProvider* provider = g_steal_pointer(&req->provider); + if (!provider) { + provider = ai_get_provider(req->provider_name); + if (!provider) { + log_error("Provider '%s' not found for request", req->provider_name); + g_free(req->provider_name); + g_free(req->request_url); + g_free(req); + return NULL; + } + ai_provider_ref(provider); + } CURL* curl = curl_easy_init(); if (!curl) { log_error("Failed to initialize curl for %s", req->provider_name); + ai_provider_unref(provider); g_free(req->provider_name); g_free(req->request_url); g_free(req); @@ -901,6 +1094,7 @@ _ai_generic_request_thread(gpointer data) auto_gchar gchar* error_msg = g_strdup_printf("No API key set for provider '%s'.", req->provider_name); _aiwin_display_error(req->user_data, error_msg); curl_easy_cleanup(curl); + ai_provider_unref(provider); g_free(req->provider_name); g_free(req->request_url); g_free(req); @@ -1247,19 +1441,11 @@ static gpointer _models_fetch_thread(gpointer data) { ai_request_t* req = (ai_request_t*)data; - AIProvider* provider = ai_get_provider(req->provider_name); - if (!provider) { - g_free(req->provider_name); - g_free(req); - return NULL; - } - /* Build URL for listing models */ - auto_gchar gchar* base_url = g_strdup_printf("%s%s", provider->api_url, - g_str_has_suffix(provider->api_url, "/") ? "" : "/"); - auto_gchar gchar* request_url = g_strdup_printf("%sv1/models", base_url); - - req->request_url = g_strdup(request_url); + /* req->provider carries a ref taken on the main thread, so the provider + * cannot be freed under us; the locked snapshot guards the URL swap */ + auto_gchar gchar* base_url = _provider_base_url(req->provider, NULL); + req->request_url = g_strdup_printf("%sv1/models", base_url); req->handler = _models_response_handler; return _ai_generic_request_thread(req); @@ -1277,13 +1463,17 @@ ai_fetch_models(const gchar* provider_name, gpointer user_data) return FALSE; } - /* Always fetch fresh models from API */ + /* Always fetch fresh models from API. Ref the provider on this thread, + * while it is guaranteed alive, so /ai remove provider cannot free it + * under the worker; the worker's request struct owns the ref. */ ai_request_t* req = g_new0(ai_request_t, 1); req->provider_name = g_strdup(provider_name); + req->provider = ai_provider_ref(provider); req->user_data = user_data; GThread* thread = g_thread_new("ai_models_fetch", _models_fetch_thread, req); if (!thread) { + ai_provider_unref(req->provider); g_free(req->provider_name); g_free(req); log_error("Failed to create thread for model fetch"); @@ -1464,28 +1654,20 @@ ai_session_switch(AISession* session, const gchar* provider_name, * ======================================================================== */ /** - * Build JSON payload from a GList of AIMessage nodes and a prompt string. - * This is the thread-safe variant used by _ai_request_thread() which holds - * the session lock while calling it. + * Serialize history plus the new user prompt as a JSON message-object list + * (without the enclosing brackets); the fragment is shared by both API + * flavours. Must be called with the session lock held. * - * Custom provider settings are merged into the payload as additional JSON - * key-value pairs alongside "model" and "messages": scalar values - * (number/true/false/null) are emitted bare, anything else as a JSON string; - * reserved keys (model/messages/stream) are skipped. - * - * @param provider The AI provider (for custom settings) - * @param model The model identifier * @param history GList of AIMessage* (must be held under session lock) * @param prompt The new user prompt to append - * @return Newly allocated JSON string (caller must free) + * @return Newly allocated fragment (caller must free) */ static gchar* -_build_json_payload_from_list(AIProvider* provider, const gchar* model, GList* history, const gchar* prompt) +_serialize_message_list(GList* history, const gchar* prompt) { GString* messages_json = g_string_new(""); - GList* curr = history; - while (curr) { + for (GList* curr = history; curr; curr = g_list_next(curr)) { AIMessage* msg = curr->data; auto_gchar gchar* escaped_content = ai_json_escape(msg->content); auto_gchar gchar* escaped_role = ai_json_escape(msg->role); @@ -1495,8 +1677,6 @@ _build_json_payload_from_list(AIProvider* provider, const gchar* model, GList* h } g_string_append_printf(messages_json, "{\"role\":\"%s\",\"content\":\"%s\"}", escaped_role, escaped_content); - - curr = g_list_next(curr); } /* Add the new user message */ @@ -1506,11 +1686,24 @@ _build_json_payload_from_list(AIProvider* provider, const gchar* model, GList* h } g_string_append_printf(messages_json, "{\"role\":\"user\",\"content\":\"%s\"}", escaped_prompt); - auto_gchar gchar* escaped_model = ai_json_escape(model); + return g_string_free(messages_json, FALSE); +} - /* Build custom settings JSON fragment; settings is mutated on the main - * thread (/ai set custom), so iterate under settings_lock */ +/** + * Serialize custom provider settings as a JSON key-value fragment: scalar + * values (number/true/false/null) are emitted bare, anything else as a JSON + * string; reserved keys (model/messages/input/stream) are skipped. + * + * @param provider The AI provider (may be NULL) + * @param store_set Output: set TRUE when a custom "store" value is present + * @return Newly allocated fragment, empty string when there are no settings + */ +static gchar* +_serialize_custom_settings(AIProvider* provider, gboolean* store_set) +{ GString* custom_json = g_string_new(""); + *store_set = FALSE; + if (provider) { pthread_mutex_lock(&provider->settings_lock); if (provider->settings) { @@ -1521,6 +1714,9 @@ _build_json_payload_from_list(AIProvider* provider, const gchar* model, GList* h if (_is_reserved_payload_key((gchar*)key)) { continue; /* fixed payload fields stay authoritative */ } + if (g_strcmp0((gchar*)key, "store") == 0) { + *store_set = TRUE; /* suppresses the fixed "store":false */ + } auto_gchar gchar* escaped_key = ai_json_escape((gchar*)key); if (custom_json->len > 0) { g_string_append_c(custom_json, ','); @@ -1537,31 +1733,141 @@ _build_json_payload_from_list(AIProvider* provider, const gchar* model, GList* h pthread_mutex_unlock(&provider->settings_lock); } - /* Assemble final payload: model, messages, custom settings, then fixed keys */ - gchar* json_payload; - if (custom_json->len > 0) { - json_payload = g_strdup_printf( - "{\"model\":\"%s\",\"messages\":[%s],%s,\"stream\":false}", - escaped_model, messages_json->str, custom_json->str); - } else { - json_payload = g_strdup_printf( - "{\"model\":\"%s\",\"messages\":[%s],\"stream\":false}", - escaped_model, messages_json->str); + return g_string_free(custom_json, FALSE); +} + +/** + * Assemble the payload for one API flavour from pre-serialized fragments: + * model, message list, custom settings, then fixed keys. Chat completions + * uses the "messages" list key; responses uses "input" plus "store":false + * unless a custom "store" setting overrides it. + * + * @param model The model identifier + * @param messages_json Fragment from _serialize_message_list() + * @param custom_json Fragment from _serialize_custom_settings() + * @param store_set Whether custom_json already carries a "store" value + * @param api_type The API flavour to build for (must not be AUTO) + * @return Newly allocated JSON string (caller must free) + */ +static gchar* +_assemble_json_payload(const gchar* model, const gchar* messages_json, const gchar* custom_json, gboolean store_set, AIApiType api_type) +{ + auto_gchar gchar* escaped_model = ai_json_escape(model); + const gchar* list_key = api_type == AI_API_TYPE_CHAT_COMPLETIONS ? "messages" : "input"; + const gchar* fixed_tail = (api_type == AI_API_TYPE_CHAT_COMPLETIONS || store_set) + ? "\"stream\":false" + : "\"stream\":false,\"store\":false"; + + return g_strdup_printf("{\"model\":\"%s\",\"%s\":[%s],%s%s%s}", + escaped_model, list_key, messages_json, + custom_json, *custom_json ? "," : "", fixed_tail); +} + +/* Chat completions: {"choices":[{"message":{"content":"..."}}]}. Anchor on + * "message" so a non-string "content" ordered earlier in the choice (e.g. + * "logprobs":{"content":[...]}) is not mistaken for the reply. */ +static gchar* +_parse_chat_completions(const gchar* json) +{ + const gchar* choices = strstr(json, "\"choices\""); + if (!choices) + return NULL; + const gchar* message = strstr(choices, "\"message\""); + return _extract_json_string(message ? message : choices, "content"); +} + +/* TRUE when no object boundary ({ or }) lies between @from and @to outside of + * string literals — i.e. both positions belong to the same JSON object */ +static gboolean +_same_json_object(const gchar* from, const gchar* to) +{ + gboolean in_string = FALSE; + for (const gchar* p = from; p < to && *p; p++) { + if (in_string) { + if (*p == '\\' && *(p + 1) != '\0') + p++; + else if (*p == '"') + in_string = FALSE; + } else if (*p == '"') { + in_string = TRUE; + } else if (*p == '{' || *p == '}') { + return FALSE; + } + } + return TRUE; +} + +/* Responses: {"output":[...,{"content":[{"type":"output_text","text":"..."}]}]}. + * "content" is an array here, so extract the inner "text" of the output_text + * part; reasoning items carry their own "text" (summary) ahead of it. */ +static gchar* +_parse_responses(const gchar* json) +{ + const gchar* output = strstr(json, "\"output\""); + if (!output) + return NULL; + + const gchar* part = strstr(output, "\"output_text\""); + if (part) { + gchar* text = _extract_json_string(part, "text"); + if (text) + return text; + /* "text" may precede "type":"output_text" in the part: take the + * nearest preceding "text" that shares the part's object, so an + * earlier item's text (a reasoning summary) is never returned; skip + * matches where extraction fails (e.g. a string value equal to "text") */ + gsize window = part - output; + while (window > 0) { + const gchar* key = g_strrstr_len(output, window, "\"text\""); + if (!key) + break; + if (_same_json_object(key, part)) { + text = _extract_json_string(key, "text"); + if (text) + return text; + } + window = key - output; + } + return NULL; } - g_string_free(messages_json, TRUE); - g_string_free(custom_json, TRUE); - return json_payload; + /* No output_text tag (truncated reply, or a server tagging the part + * "type":"text"): anchor on the message "content" array so a reasoning + * "summary" text is not returned as the reply */ + const gchar* content_arr = strstr(output, "\"content\""); + return _extract_json_string(content_arr ? content_arr : output, "text"); +} + +/* Extract assistant content, trying the known request flavour's extractor + * first (AUTO means chat-completions first), then the other flavour, then a + * bare top-level "content" string. Each extractor runs at most once. */ +gchar* +ai_parse_response_typed(AIApiType api_type, const gchar* response_json) +{ + if (!response_json || strlen(response_json) == 0) + return NULL; + + gchar* out; + if (api_type == AI_API_TYPE_RESPONSES) { + out = _parse_responses(response_json); + if (!out) + out = _parse_chat_completions(response_json); + } else { + out = _parse_chat_completions(response_json); + if (!out) + out = _parse_responses(response_json); + } + if (out) + return out; + + /* No recognizable envelope: accept a bare top-level "content" string */ + return _extract_json_string(response_json, "content"); } gchar* ai_parse_response(const gchar* response_json) { - if (!response_json || strlen(response_json) == 0) - return NULL; - - /* OpenAI /v1/chat/completions: {"choices":[{"message":{"content":"..."}}]} */ - return _extract_json_string(response_json, "content"); + return ai_parse_response_typed(AI_API_TYPE_AUTO, response_json); } /* Extract the human-readable error.message from a provider error envelope: @@ -1636,10 +1942,11 @@ _ai_request_thread(gpointer data) return NULL; } - /* Build JSON payload from history + prompt (under lock), then add user message to history */ + /* Serialize the payload fragments once from a single history snapshot + * (under lock); per-flavour envelopes are assembled cheaply per attempt */ log_debug("[AI-THREAD] Building JSON payload..."); pthread_mutex_lock(&session->lock); - auto_gchar gchar* json_payload = _build_json_payload_from_list(local_provider, local_model, session->history, prompt); + auto_gchar gchar* messages_json = _serialize_message_list(session->history, prompt); /* Add user message to history now (it's in JSON but not yet in history) */ AIMessage* user_msg = g_new0(AIMessage, 1); @@ -1648,57 +1955,122 @@ _ai_request_thread(gpointer data) session->history = g_list_append(session->history, user_msg); pthread_mutex_unlock(&session->lock); log_debug("[AI-THREAD] Added user message to history"); - log_debug("[AI-THREAD] JSON payload: %s", json_payload); + + gboolean store_set = FALSE; + auto_gchar gchar* custom_json = _serialize_custom_settings(local_provider, &store_set); /* Build headers using shared helper */ struct curl_slist* headers = _build_curl_headers(local_provider, local_api_key); - /* Response buffer */ + /* Response buffer (allocated per attempt inside the loop) */ struct curl_response_t response; - response.data = g_new0(gchar, 1); + response.data = NULL; response.size = 0; - /* Build request URL */ - const gchar* api_url = local_provider->api_url; - auto_gchar gchar* request_url = g_strdup_printf("%s%sv1/chat/completions", api_url, g_str_has_suffix(api_url, "/") ? "" : "/"); - log_debug("[AI-THREAD] API URL: %s", api_url); - log_debug("[AI-THREAD] API Request URL: %s", request_url); + /* Snapshot the URL together with the epoch, so a flavour stamped after + * this request cannot be attributed to a URL it was not probed against */ + gint api_epoch = 0; + auto_gchar gchar* base_url = _provider_base_url(local_provider, &api_epoch); + + /* Flavour(s) to try: explicit setting wins; a previously detected flavour + * is a hint that keeps the other flavour as a fallback attempt (so a + * backend change self-corrects in-request); else probe responses first. */ + AIApiType configured = g_atomic_int_get(&local_provider->api_type); + AIApiType attempts[2]; + gint n_attempts = 0; + if (configured != AI_API_TYPE_AUTO) { + attempts[n_attempts++] = configured; + } else { + AIApiType resolved = g_atomic_int_get(&local_provider->resolved_api_type); + AIApiType first = resolved != AI_API_TYPE_AUTO ? resolved : AI_API_TYPE_RESPONSES; + attempts[n_attempts++] = first; + attempts[n_attempts++] = _other_flavour(first); + } + log_debug("[AI-THREAD] API URL: %s", base_url); log_debug("[AI-THREAD] Model: %s", local_model); - /* Set URL and execute request */ - curl_easy_setopt(curl, CURLOPT_URL, request_url); + /* Per-request options shared by all attempts */ curl_easy_setopt(curl, CURLOPT_HTTPHEADER, headers); curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, _write_callback); curl_easy_setopt(curl, CURLOPT_WRITEDATA, &response); - curl_easy_setopt(curl, CURLOPT_POSTFIELDS, json_payload); curl_easy_setopt(curl, CURLOPT_TIMEOUT, 60L); - CURLcode res = curl_easy_perform(curl); - log_debug("[AI-THREAD] curl_easy_perform completed, res: %d", res); - - /* Get HTTP response code */ + CURLcode res = CURLE_OK; long http_code = 0; - curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &http_code); - log_debug("[AI-THREAD] HTTP response code: %ld", http_code); + AIApiType used_type = attempts[0]; + /* A payload rejection (400/422) that triggered the fallback is the + * actionable error; keep it in case the other flavour also fails */ + auto_gchar gchar* first_reject_body = NULL; + long first_reject_code = 0; + for (gint i = 0; i < n_attempts; i++) { + used_type = attempts[i]; + auto_gchar gchar* request_url = g_strdup_printf("%s%s", base_url, _api_type_endpoint(used_type)); + auto_gchar gchar* json_payload = _assemble_json_payload(local_model, messages_json, custom_json, store_set, used_type); + log_debug("[AI-THREAD] API Request URL: %s", request_url); + log_debug("[AI-THREAD] JSON payload: %s", json_payload); + + /* Reset the response buffer for this attempt */ + g_free(response.data); + response.data = g_new0(gchar, 1); + response.size = 0; + + curl_easy_setopt(curl, CURLOPT_URL, request_url); + curl_easy_setopt(curl, CURLOPT_COPYPOSTFIELDS, json_payload); + + res = curl_easy_perform(curl); + log_debug("[AI-THREAD] curl_easy_perform completed, res: %d", res); + + http_code = 0; + curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &http_code); + log_debug("[AI-THREAD] HTTP response code: %ld", http_code); + + if (res == CURLE_OK && i + 1 < n_attempts && _should_try_other_flavour(http_code, response.data)) { + log_info("[AI-THREAD] %s endpoint rejected request for '%s' (HTTP %ld), falling back to %s", + ai_api_type_to_string(used_type), local_provider_name, http_code, + ai_api_type_to_string(attempts[i + 1])); + log_debug("[AI-THREAD] Rejected %s response body: %s", + ai_api_type_to_string(used_type), response.data); + if (!first_reject_body && !_is_endpoint_missing(http_code, response.data)) { + first_reject_body = g_strdup(response.data); + first_reject_code = http_code; + } + continue; + } + break; + } auto_gchar gchar* response_data = g_steal_pointer(&response.data); + if (configured == AI_API_TYPE_AUTO && res == CURLE_OK && _is_endpoint_missing(http_code, response_data)) { + /* The flavour we tried is no longer served; re-probe on the next request */ + _provider_set_resolved(local_provider, AI_API_TYPE_AUTO, api_epoch); + } + if (res != CURLE_OK) { auto_gchar gchar* error_msg = g_strdup(curl_easy_strerror(res)); log_error("AI request failed for %s/%s: %s", local_provider_name, local_model, error_msg); _aiwin_display_error(user_data, error_msg); } else if (http_code >= 400) { - /* Handle HTTP errors */ + /* Handle HTTP errors; a payload rejection that triggered the fallback + * is more actionable than the other endpoint's follow-up failure */ + const gchar* err_body = first_reject_body ? first_reject_body : response_data; + long err_code = first_reject_body ? first_reject_code : http_code; log_debug("[AI-THREAD] HTTP error response body (%zu bytes): %s", response.size, response_data); /* Try to extract the actual error message from the JSON response */ - auto_gchar gchar* parsed_error = ai_parse_error_message(response_data); - auto_gchar gchar* error_msg = parsed_error ? g_strdup_printf("HTTP %ld: %s", http_code, parsed_error) : g_strdup_printf("HTTP %ld: %s", http_code, response_data && strlen(response_data) > 0 ? response_data : "Unknown error"); + auto_gchar gchar* parsed_error = ai_parse_error_message(err_body); + auto_gchar gchar* error_msg = parsed_error ? g_strdup_printf("HTTP %ld: %s", err_code, parsed_error) : g_strdup_printf("HTTP %ld: %s", err_code, err_body && strlen(err_body) > 0 ? err_body : "Unknown error"); log_error("AI request failed for %s/%s: %s", local_provider_name, local_model, error_msg); _aiwin_display_error(user_data, error_msg); } else { log_debug("[AI-THREAD] Raw API response (%zu bytes): %s", response.size, response_data); - auto_gchar gchar* content = ai_parse_response(response_data); + auto_gchar gchar* content = ai_parse_response_typed(used_type, response_data); if (content) { + if (configured == AI_API_TYPE_AUTO && http_code >= 200 && http_code < 300) { + /* Remember the flavour that worked so later requests try it + * first; the epoch check discards the stamp if the URL or + * api-type changed while this request was in flight */ + _provider_set_resolved(local_provider, used_type, api_epoch); + } /* Add assistant response to history (under lock) */ pthread_mutex_lock(&session->lock); AIMessage* assistant_msg = g_new0(AIMessage, 1); @@ -1708,6 +2080,10 @@ _ai_request_thread(gpointer data) pthread_mutex_unlock(&session->lock); _aiwin_display_response(user_data, content); } else { + /* A 2xx we cannot parse means the cached flavour may be wrong; drop + * it so the next request re-probes instead of wedging on it */ + if (configured == AI_API_TYPE_AUTO) + _provider_set_resolved(local_provider, AI_API_TYPE_AUTO, api_epoch); log_error("AI response parse failed for %s/%s: %.200s...", local_provider_name, local_model, response_data); _aiwin_display_error(user_data, "Failed to parse AI response."); diff --git a/src/ai/ai_client.h b/src/ai/ai_client.h index 7898dbfc..f29dddd5 100644 --- a/src/ai/ai_client.h +++ b/src/ai/ai_client.h @@ -12,6 +12,21 @@ typedef struct ai_message_t gchar* content; /* Message content */ } AIMessage; +/** + * @brief API flavour used to talk to a provider. + * + * AUTO tries /v1/responses first and falls back to /v1/chat/completions when + * the endpoint is missing (404/405/501) or rejects the payload shape + * (400/422). The working flavour is cached for the rest of the run as a + * first-attempt hint (the other flavour stays available as a fallback). The + * other two values pin the flavour explicitly. + */ +typedef enum { + AI_API_TYPE_AUTO = 0, + AI_API_TYPE_RESPONSES, + AI_API_TYPE_CHAT_COMPLETIONS, +} AIApiType; + /** * @brief AI provider configuration. */ @@ -21,7 +36,10 @@ typedef struct ai_provider_t gchar* api_url; /* API endpoint URL */ gchar* project_id; /* Optional project ID (for some providers) */ gchar* default_model; /* Default model for this provider */ - pthread_mutex_t settings_lock; /* Protects settings (iterated by the request thread) */ + gint api_type; /* Configured AIApiType (atomic access; persisted) */ + gint resolved_api_type; /* AIApiType detected via 404 fallback (atomic access; runtime cache) */ + gint api_epoch; /* Bumped on URL change (atomic); a stale epoch discards resolved_api_type stamps */ + pthread_mutex_t settings_lock; /* Protects settings and api_url (read by request threads) */ GHashTable* settings; /* Extensible per-provider settings (e.g., temperature=0.7) */ GList* models; /* Cached models (gchar*) */ gboolean models_fresh; /* Whether models cache is current */ @@ -67,7 +85,7 @@ AIProvider* ai_get_provider(const gchar* name); * Add or update a provider configuration. * @param name The provider name * @param api_url The API endpoint URL - * @return New AIProvider* (caller must unref when done) + * @return Borrowed AIProvider* owned by the provider table (do not unref) */ AIProvider* ai_add_provider(const gchar* name, const gchar* api_url); @@ -141,9 +159,39 @@ void ai_set_provider_default_model(const gchar* provider_name, const gchar* mode */ const gchar* ai_get_provider_default_model(const gchar* provider_name); +/** + * Set the API type for a provider. + * @param provider_name The provider name + * @param api_type "responses", "chat-completions", or "auto" (detect by probing) + * @return TRUE on success, FALSE on unknown provider or invalid type string + */ +gboolean ai_set_provider_api_type(const gchar* provider_name, const gchar* api_type); + +/** + * Get the configured API type for a provider. + * @param provider_name The provider name + * @return The configured AIApiType (AI_API_TYPE_AUTO for unknown providers) + */ +AIApiType ai_get_provider_api_type(const gchar* provider_name); + +/** + * Parse an API type string ("auto", "responses", "chat-completions"). + * @param str The string to parse + * @param type Output for the parsed value (untouched on failure) + * @return TRUE if str is a valid API type + */ +gboolean ai_api_type_from_string(const gchar* str, AIApiType* type); + +/** + * String form of an AIApiType, e.g. for display. + * @param type The API type + * @return Static string, never NULL + */ +const gchar* ai_api_type_to_string(AIApiType type); + /** * Set a custom setting for a provider. - * Reserved payload keys (model, messages, stream) are rejected. + * Reserved payload keys (model, messages, input, stream) are rejected. * @param provider_name The provider name * @param setting The setting key (e.g., "temperature") * @param value The setting value, or NULL to remove @@ -187,14 +235,25 @@ gboolean ai_models_are_fresh(const gchar* provider_name); void ai_parse_models_from_json(AIProvider* provider, const gchar* json); /** - * Extract assistant content from an OpenAI chat-completions response body - * ({"choices":[{"message":{"content":"..."}}]}). Decodes the \" and \n - * escape sequences only. + * Extract assistant content from a provider response body. Detects the + * flavour from the envelope: chat completions + * ({"choices":[{"message":{"content":"..."}}]}) or responses + * ({"output":[{"content":[{"type":"output_text","text":"..."}]}]}). * @param response_json The JSON body from the provider * @return Newly allocated content string, or NULL if not found (caller frees) */ gchar* ai_parse_response(const gchar* response_json); +/** + * Extract assistant content trying the known request flavour's extractor + * first, then the other flavour, then a bare top-level "content" string. + * ai_parse_response is equivalent to passing AI_API_TYPE_AUTO. + * @param api_type The API flavour the response was requested with + * @param response_json The JSON body from the provider + * @return Newly allocated content string, or NULL if not found (caller frees) + */ +gchar* ai_parse_response_typed(AIApiType api_type, const gchar* response_json); + /** * Extract human-readable error message from an API error envelope. * Expected format: {"error":{"message":"...","type":"...","code":...}} diff --git a/src/command/cmd_ac.c b/src/command/cmd_ac.c index 99c2ac36..3bcb2022 100644 --- a/src/command/cmd_ac.c +++ b/src/command/cmd_ac.c @@ -264,6 +264,7 @@ static Autocomplete avatar_ac; static Autocomplete ai_subcommands_ac; static Autocomplete ai_set_subcommands_ac; static Autocomplete ai_set_custom_subcommands_ac; +static Autocomplete ai_api_types_ac; static Autocomplete ai_remove_subcommands_ac; static Autocomplete url_ac; static Autocomplete executable_ac; @@ -453,6 +454,7 @@ static Autocomplete* all_acs[] = { &ai_subcommands_ac, &ai_set_subcommands_ac, &ai_set_custom_subcommands_ac, + &ai_api_types_ac, &ai_remove_subcommands_ac, &ai_models_ac }; @@ -1183,8 +1185,13 @@ cmd_ac_init(void) autocomplete_add_unsorted(ai_set_subcommands_ac, "provider", FALSE); autocomplete_add_unsorted(ai_set_subcommands_ac, "token", FALSE); autocomplete_add_unsorted(ai_set_subcommands_ac, "default-model", FALSE); + autocomplete_add_unsorted(ai_set_subcommands_ac, "api-type", FALSE); autocomplete_add_unsorted(ai_set_subcommands_ac, "custom", FALSE); + autocomplete_add_unsorted(ai_api_types_ac, "chat-completions", FALSE); + autocomplete_add_unsorted(ai_api_types_ac, "responses", FALSE); + autocomplete_add_unsorted(ai_api_types_ac, "auto", FALSE); + autocomplete_add_unsorted(ai_set_custom_subcommands_ac, "tools", FALSE); autocomplete_add_unsorted(ai_set_custom_subcommands_ac, "search", FALSE); autocomplete_add_unsorted(ai_set_custom_subcommands_ac, "memory", FALSE); @@ -4532,6 +4539,12 @@ _force_encryption_autocomplete(ProfWin* window, const char* const input, gboolea static char* _ai_provider_and_model_autocomplete(ProfWin* window, const char* const input, gboolean previous, const char* cmd); +static char* +_ai_api_type_find(const char* const search_str, gboolean previous, void* context) +{ + return autocomplete_complete(ai_api_types_ac, search_str, FALSE, previous); +} + static char* _ai_autocomplete(ProfWin* window, const char* const input, gboolean previous) { @@ -4560,6 +4573,18 @@ _ai_autocomplete(ProfWin* window, const char* const input, gboolean previous) return result; } + // /ai set api-type - autocomplete the type value + result = autocomplete_param_no_with_func(input, "/ai set api-type", 5, _ai_api_type_find, previous, NULL); + if (result) { + return result; + } + + // /ai set api-type - autocomplete provider names + result = autocomplete_param_with_func(input, "/ai set api-type", ai_providers_find, previous, NULL); + if (result) { + return result; + } + // /ai set custom - autocomplete provider names result = autocomplete_param_with_func(input, "/ai set custom", ai_providers_find, previous, NULL); if (result) { diff --git a/src/command/cmd_defs.c b/src/command/cmd_defs.c index 795d0547..304b4b8f 100644 --- a/src/command/cmd_defs.c +++ b/src/command/cmd_defs.c @@ -2815,6 +2815,7 @@ static const struct cmd_t command_defs[] = { "/ai set provider ", "/ai set token ", "/ai set default-model ", + "/ai set api-type chat-completions|responses|auto", "/ai set custom ", "/ai remove provider ", "/ai providers", @@ -2830,9 +2831,10 @@ static const struct cmd_t command_defs[] = { "Chat history is maintained per session and not persisted locally.") CMD_ARGS( { "", "Display current AI settings and configured providers" }, - { "set provider ", "Add or update a provider with custom API endpoint" }, + { "set provider ", "Add or update a provider with a custom API base URL (endpoint paths are appended automatically)" }, { "set token ", "Set API token for a provider (e.g., openai, perplexity)" }, { "set default-model ", "Set default model for a provider" }, + { "set api-type chat-completions|responses|auto", "Pin the provider API flavour; auto tries responses and falls back to chat completions when the endpoint is missing" }, { "set custom ", "Set provider-level setting (e.g., temperature, max_tokens)" }, { "remove provider ", "Remove a custom provider" }, { "providers", "List configured providers with full details" }, @@ -2844,8 +2846,9 @@ static const struct cmd_t command_defs[] = { "/ai", "/ai set token openai sk-xxx", "/ai set token perplexity pplx-xxx", - "/ai set provider custom https://my-api.com/v1/chat/completions", + "/ai set provider custom https://my-api.com/", "/ai set default-model perplexity sonar", + "/ai set api-type perplexity responses", "/ai set custom perplexity temperature 0.7", "/ai remove provider custom", "/ai start", diff --git a/src/command/cmd_funcs.c b/src/command/cmd_funcs.c index d9293e66..898925d3 100644 --- a/src/command/cmd_funcs.c +++ b/src/command/cmd_funcs.c @@ -10703,6 +10703,19 @@ cmd_vcard_save(ProfWin* window, const char* const command, gchar** args) return TRUE; } +/* Show the provider's API flavour: pinned, or auto with the detected one */ +static void +_cons_show_ai_api_type(AIProvider* provider) +{ + AIApiType api_type = g_atomic_int_get(&provider->api_type); + AIApiType resolved = g_atomic_int_get(&provider->resolved_api_type); + if (api_type != AI_API_TYPE_AUTO) { + cons_show(" API type: %s", ai_api_type_to_string(api_type)); + } else if (resolved != AI_API_TYPE_AUTO) { + cons_show(" API type: auto (detected: %s)", ai_api_type_to_string(resolved)); + } +} + gboolean cmd_ai(ProfWin* window, const char* const command, gchar** args) { @@ -10724,6 +10737,7 @@ cmd_ai(ProfWin* window, const char* const command, gchar** args) if (default_model) { cons_show(" Default model: %s", default_model); } + _cons_show_ai_api_type(provider); if (provider->models && g_list_length(provider->models) > 0) { cons_show(" Cached models: %d", g_list_length(provider->models)); } @@ -10781,6 +10795,19 @@ cmd_ai_set(ProfWin* window, const char* const command, gchar** args) cons_show("Default model for provider '%s' set to: %s", args[2], args[3]); cons_show(""); return TRUE; + } else if (g_strcmp0(args[1], "api-type") == 0) { + // /ai set api-type chat-completions|responses|auto + if (g_strv_length(args) < 4) { + cons_bad_cmd_usage(command); + return TRUE; + } + if (ai_set_provider_api_type(args[2], args[3])) { + cons_show("API type for provider '%s' set to: %s", args[2], args[3]); + } else { + cons_show_error("Cannot set API type for provider '%s': unknown provider or invalid type (use chat-completions, responses, or auto).", args[2]); + } + cons_show(""); + return TRUE; } else if (g_strcmp0(args[1], "custom") == 0) { // /ai set custom if (g_strv_length(args) < 5) { @@ -10790,7 +10817,7 @@ cmd_ai_set(ProfWin* window, const char* const command, gchar** args) if (ai_set_provider_setting(args[2], args[3], args[4])) { cons_show("Setting '%s' for provider '%s' set to: %s", args[3], args[2], args[4]); } else { - cons_show_error("Cannot set '%s' for provider '%s': unknown provider or reserved key (model, messages, stream).", args[3], args[2]); + cons_show_error("Cannot set '%s' for provider '%s': unknown provider or reserved key (model, messages, input, stream).", args[3], args[2]); } cons_show(""); return TRUE; @@ -11105,6 +11132,7 @@ cmd_ai_providers(ProfWin* window, const char* const command, gchar** args) cons_show(" %s", provider->name); cons_show(" URL: %s", provider->api_url); cons_show(" Key: %s", key ? "configured" : "NOT configured"); + _cons_show_ai_api_type(provider); cons_show(""); } g_list_free(providers); diff --git a/src/config/preferences.c b/src/config/preferences.c index 04d3dda5..708386c0 100644 --- a/src/config/preferences.c +++ b/src/config/preferences.c @@ -2002,6 +2002,30 @@ prefs_ai_get_default_model(const char* const provider) return _ai_get_string(provider, "default_model"); } +/* --- API type --- */ + +gboolean +prefs_ai_set_api_type(const char* const provider, const char* const api_type) +{ + if (!provider || !api_type || !prefs) + return FALSE; + if (api_type[0] == '\0') + return prefs_ai_remove_api_type(provider); + return _ai_set_string(provider, "api_type", api_type); +} + +gboolean +prefs_ai_remove_api_type(const char* const provider) +{ + return _ai_remove_string(provider, "api_type"); +} + +char* +prefs_ai_get_api_type(const char* const provider) +{ + return _ai_get_string(provider, "api_type"); +} + /* --- Provider URL --- */ gboolean diff --git a/src/config/preferences.h b/src/config/preferences.h index 4e422a05..3631d311 100644 --- a/src/config/preferences.h +++ b/src/config/preferences.h @@ -368,6 +368,11 @@ gboolean prefs_ai_set_default_model(const char* const provider, const char* cons gboolean prefs_ai_remove_default_model(const char* const provider); char* prefs_ai_get_default_model(const char* const provider); +/* AI API type per provider ("responses" or "chat-completions") */ +gboolean prefs_ai_set_api_type(const char* const provider, const char* const api_type); +gboolean prefs_ai_remove_api_type(const char* const provider); +char* prefs_ai_get_api_type(const char* const provider); + /* AI per-provider custom settings (e.g. tools, search) — persisted as * setting.= inside [ai/]. Pass NULL value to remove. */ gboolean prefs_ai_set_setting(const char* const provider, const char* const setting, const char* const value); diff --git a/tests/unittests/test_ai_client.c b/tests/unittests/test_ai_client.c index 516fcb7f..aa6205b4 100644 --- a/tests/unittests/test_ai_client.c +++ b/tests/unittests/test_ai_client.c @@ -62,12 +62,10 @@ test_ai_add_provider(void** state) assert_string_equal("custom", provider->name); assert_string_equal("https://custom.api.com/v1", provider->api_url); - /* Update existing provider (returns ref; caller owns it) */ + /* Update existing provider (borrowed pointer, same as the create path) */ AIProvider* updated = ai_add_provider("custom", "https://new.api.com/v1"); assert_non_null(updated); assert_string_equal("https://new.api.com/v1", updated->api_url); - - ai_provider_unref(updated); } void @@ -329,11 +327,10 @@ test_ai_add_provider_update_existing(void** state) assert_non_null(provider); assert_string_equal("https://first.api.com/v1", provider->api_url); - /* Update the same provider (returns ref) */ + /* Update the same provider (borrowed pointer) */ provider = ai_add_provider("custom", "https://second.api.com/v1"); assert_non_null(provider); assert_string_equal("https://second.api.com/v1", provider->api_url); - ai_provider_unref(provider); } void @@ -622,9 +619,17 @@ test_ai_set_provider_setting_reserved_key(void** state) /* Reserved payload keys are rejected (would duplicate fixed fields) */ assert_false(ai_set_provider_setting("p_reserved_keys", "model", "x")); assert_false(ai_set_provider_setting("p_reserved_keys", "messages", "[]")); + assert_false(ai_set_provider_setting("p_reserved_keys", "input", "[]")); assert_false(ai_set_provider_setting("p_reserved_keys", "stream", "true")); assert_null(ai_get_provider_setting("p_reserved_keys", "model")); + /* "store" is a legitimate chat-completions parameter and overrides the + * responses-flavour "store":false default, so it stays settable */ + assert_true(ai_set_provider_setting("p_reserved_keys", "store", "true")); + auto_gchar gchar* store = ai_get_provider_setting("p_reserved_keys", "store"); + assert_non_null(store); + assert_string_equal("true", store); + /* Ordinary keys still work */ assert_true(ai_set_provider_setting("p_reserved_keys", "temperature", "0.7")); auto_gchar gchar* temp = ai_get_provider_setting("p_reserved_keys", "temperature"); @@ -632,6 +637,58 @@ test_ai_set_provider_setting_reserved_key(void** state) assert_string_equal("0.7", temp); } +void +test_ai_set_provider_api_type(void** state) +{ + ai_add_provider("p_api_type", "https://example.test/at/"); + + /* Default is auto */ + assert_int_equal(AI_API_TYPE_AUTO, ai_get_provider_api_type("p_api_type")); + + /* Pin each flavour explicitly */ + assert_true(ai_set_provider_api_type("p_api_type", "chat-completions")); + assert_int_equal(AI_API_TYPE_CHAT_COMPLETIONS, ai_get_provider_api_type("p_api_type")); + + assert_true(ai_set_provider_api_type("p_api_type", "responses")); + assert_int_equal(AI_API_TYPE_RESPONSES, ai_get_provider_api_type("p_api_type")); + + /* auto resets to detection */ + assert_true(ai_set_provider_api_type("p_api_type", "auto")); + assert_int_equal(AI_API_TYPE_AUTO, ai_get_provider_api_type("p_api_type")); + + /* Invalid values and unknown providers are rejected */ + assert_false(ai_set_provider_api_type("p_api_type", "v2")); + assert_false(ai_set_provider_api_type("p_api_type", NULL)); + assert_false(ai_set_provider_api_type("nonexistent", "responses")); + assert_false(ai_set_provider_api_type(NULL, "responses")); + + /* Unknown provider reads as auto */ + assert_int_equal(AI_API_TYPE_AUTO, ai_get_provider_api_type("nonexistent")); + assert_int_equal(AI_API_TYPE_AUTO, ai_get_provider_api_type(NULL)); +} + +void +test_ai_api_type_string_round_trip(void** state) +{ + AIApiType type = AI_API_TYPE_AUTO; + + assert_true(ai_api_type_from_string("responses", &type)); + assert_int_equal(AI_API_TYPE_RESPONSES, type); + assert_string_equal("responses", ai_api_type_to_string(type)); + + assert_true(ai_api_type_from_string("chat-completions", &type)); + assert_int_equal(AI_API_TYPE_CHAT_COMPLETIONS, type); + assert_string_equal("chat-completions", ai_api_type_to_string(type)); + + assert_true(ai_api_type_from_string("auto", &type)); + assert_int_equal(AI_API_TYPE_AUTO, type); + assert_string_equal("auto", ai_api_type_to_string(type)); + + assert_false(ai_api_type_from_string("completions", &type)); + assert_false(ai_api_type_from_string("", &type)); + assert_false(ai_api_type_from_string(NULL, &type)); +} + void test_ai_get_provider_setting(void** state) { @@ -937,12 +994,78 @@ test_ai_parse_response_openai_content(void** state) } void -test_ai_parse_response_legacy_text_format_unsupported(void** state) +test_ai_parse_response_responses_output_text(void** state) { - /* Legacy Perplexity /v1/agent format: "content" is an array, not a string. - * Dropped with the chat-completions alignment, so parsing yields NULL. */ + /* Responses API: "content" is an array of typed parts; the inner "text" + * of the output_text part is the assistant message. */ const gchar* json = "{\"output\":[{\"content\":[{\"text\":\"Search result\",\"type\":\"output_text\"}]}]}"; - assert_null(ai_parse_response(json)); + auto_gchar gchar* out = ai_parse_response(json); + assert_non_null(out); + assert_string_equal("Search result", out); +} + +void +test_ai_parse_response_responses_full_envelope(void** state) +{ + /* Realistic responses body: a reasoning item precedes the message item; + * the parser must still land on the message's output_text. */ + const gchar* json = "{\"id\":\"resp_1\",\"object\":\"response\",\"status\":\"completed\"," + "\"output\":[" + "{\"id\":\"rs_1\",\"type\":\"reasoning\",\"summary\":[]}," + "{\"id\":\"msg_1\",\"type\":\"message\",\"status\":\"completed\",\"role\":\"assistant\"," + "\"content\":[{\"type\":\"output_text\",\"annotations\":[],\"text\":\"Hello!\"}]}" + "]}"; + auto_gchar gchar* out = ai_parse_response(json); + assert_non_null(out); + assert_string_equal("Hello!", out); +} + +void +test_ai_parse_response_choices_win_over_output(void** state) +{ + /* A chat-completions envelope is authoritative even if the content + * happens to mention "output". */ + const gchar* json = "{\"choices\":[{\"message\":{\"content\":\"chat result\"}}],\"output\":[{\"content\":[{\"text\":\"wrong\"}]}]}"; + auto_gchar gchar* out = ai_parse_response(json); + assert_non_null(out); + assert_string_equal("chat result", out); +} + +void +test_ai_parse_response_responses_reasoning_summary_skipped(void** state) +{ + /* A reasoning item with a non-empty summary precedes the message item; + * the summary's "text" must not be mistaken for the assistant reply. */ + const gchar* json = "{\"output\":[" + "{\"id\":\"rs_1\",\"type\":\"reasoning\",\"summary\":[{\"type\":\"summary_text\",\"text\":\"User asks X\"}]}," + "{\"id\":\"msg_1\",\"type\":\"message\",\"status\":\"completed\",\"role\":\"assistant\"," + "\"content\":[{\"type\":\"output_text\",\"annotations\":[],\"text\":\"Actual answer\"}]}" + "]}"; + auto_gchar gchar* out = ai_parse_response(json); + assert_non_null(out); + assert_string_equal("Actual answer", out); +} + +void +test_ai_parse_response_choices_in_string_value(void** state) +{ + /* "choices" occurring as a string value must not hijack a valid + * responses envelope. */ + const gchar* json = "{\"kind\":\"choices\",\"output\":[{\"content\":[{\"type\":\"output_text\",\"text\":\"hi\"}]}]}"; + auto_gchar gchar* out = ai_parse_response(json); + assert_non_null(out); + assert_string_equal("hi", out); +} + +void +test_ai_parse_response_content_before_choices(void** state) +{ + /* Nonstandard body: a top-level content string precedes an empty + * choices array; the bare-content fallback must still find it. */ + const gchar* json = "{\"content\":\"hi\",\"choices\":[]}"; + auto_gchar gchar* out = ai_parse_response(json); + assert_non_null(out); + assert_string_equal("hi", out); } void @@ -955,6 +1078,54 @@ test_ai_parse_response_content_preferred_over_text(void** state) assert_string_equal("from content field", out); } +void +test_ai_parse_response_responses_brace_in_text(void** state) +{ + /* Responses part with "text" before "type":"output_text" and a '{' inside + * the answer: the nearest preceding "text" key must be extracted, not a + * brace inside the string value. */ + const gchar* json = "{\"output\":[{\"content\":[{\"text\":\"use { here\",\"type\":\"output_text\"}]}]}"; + auto_gchar gchar* out = ai_parse_response(json); + assert_non_null(out); + assert_string_equal("use { here", out); +} + +void +test_ai_parse_response_choices_logprobs_before_content(void** state) +{ + /* A non-string "content" (logprobs array) ordered before message.content + * must not defeat extraction; anchoring on "message" finds the reply. */ + const gchar* json = "{\"choices\":[{\"logprobs\":{\"content\":[{\"token\":\"hi\"}]}," + "\"message\":{\"role\":\"assistant\",\"content\":\"real answer\"}}]}"; + auto_gchar gchar* out = ai_parse_response(json); + assert_non_null(out); + assert_string_equal("real answer", out); +} + +void +test_ai_parse_response_reasoning_not_leaked_without_text(void** state) +{ + /* An output_text part with no "text" at all (truncated body) must yield a + * parse failure, not leak the preceding reasoning summary's text. */ + const gchar* json = "{\"output\":[" + "{\"type\":\"reasoning\",\"summary\":[{\"type\":\"summary_text\",\"text\":\"secret\"}]}," + "{\"type\":\"message\",\"content\":[{\"type\":\"output_text\",\"annotations\":[]}]}" + "]}"; + gchar* out = ai_parse_response(json); + assert_null(out); +} + +void +test_ai_parse_response_responses_text_value_text(void** state) +{ + /* A reply that is exactly the string "text" must not confuse the + * backscan (the value's own quoted bytes match the key needle). */ + const gchar* json = "{\"output\":[{\"content\":[{\"text\":\"text\",\"type\":\"output_text\"}]}]}"; + auto_gchar gchar* out = ai_parse_response(json); + assert_non_null(out); + assert_string_equal("text", out); +} + void test_ai_parse_response_escaped_quote(void** state) { @@ -1492,6 +1663,26 @@ test_ai_prefs_round_trip_remove_key(void** state) assert_null(after); } +void +test_ai_prefs_round_trip_api_type(void** state) +{ + ai_add_provider("p_persist_api_type", "https://example.test/pat/"); + assert_true(ai_set_provider_api_type("p_persist_api_type", "chat-completions")); + + ai_client_shutdown(); + ai_client_init(); + + assert_int_equal(AI_API_TYPE_CHAT_COMPLETIONS, ai_get_provider_api_type("p_persist_api_type")); + + /* Setting auto removes the persisted value */ + assert_true(ai_set_provider_api_type("p_persist_api_type", "auto")); + + ai_client_shutdown(); + ai_client_init(); + + assert_int_equal(AI_API_TYPE_AUTO, ai_get_provider_api_type("p_persist_api_type")); +} + void test_ai_prefs_multiple_providers_persist(void** state) { diff --git a/tests/unittests/test_ai_client.h b/tests/unittests/test_ai_client.h index 6bf2062e..6b274475 100644 --- a/tests/unittests/test_ai_client.h +++ b/tests/unittests/test_ai_client.h @@ -44,6 +44,8 @@ void test_ai_set_provider_default_model(void** state); void test_ai_get_provider_default_model(void** state); void test_ai_set_provider_setting(void** state); void test_ai_set_provider_setting_reserved_key(void** state); +void test_ai_set_provider_api_type(void** state); +void test_ai_api_type_string_round_trip(void** state); void test_ai_get_provider_setting(void** state); /* Model caching tests */ void test_ai_models_are_fresh_initial(void** state); @@ -65,8 +67,17 @@ int ai_client_teardown_with_prefs(void** state); /* Chat response parser tests (ai_parse_response) */ void test_ai_parse_response_openai_content(void** state); -void test_ai_parse_response_legacy_text_format_unsupported(void** state); +void test_ai_parse_response_responses_output_text(void** state); +void test_ai_parse_response_responses_full_envelope(void** state); +void test_ai_parse_response_responses_reasoning_summary_skipped(void** state); +void test_ai_parse_response_choices_win_over_output(void** state); +void test_ai_parse_response_choices_in_string_value(void** state); +void test_ai_parse_response_content_before_choices(void** state); void test_ai_parse_response_content_preferred_over_text(void** state); +void test_ai_parse_response_responses_brace_in_text(void** state); +void test_ai_parse_response_choices_logprobs_before_content(void** state); +void test_ai_parse_response_reasoning_not_leaked_without_text(void** state); +void test_ai_parse_response_responses_text_value_text(void** state); void test_ai_parse_response_escaped_quote(void** state); void test_ai_parse_response_newline_escape(void** state); void test_ai_parse_response_empty_content(void** state); @@ -129,6 +140,7 @@ void test_ai_parse_models_multiple_models(void** state); /* Prefs round-trip tests */ void test_ai_prefs_round_trip_api_key(void** state); void test_ai_prefs_round_trip_remove_key(void** state); +void test_ai_prefs_round_trip_api_type(void** state); void test_ai_prefs_multiple_providers_persist(void** state); /* Autocomplete deeper coverage */ diff --git a/tests/unittests/unittests.c b/tests/unittests/unittests.c index cf01788e..523a2321 100644 --- a/tests/unittests/unittests.c +++ b/tests/unittests/unittests.c @@ -753,6 +753,8 @@ main(int argc, char* argv[]) cmocka_unit_test_setup_teardown(test_ai_get_provider_default_model, ai_client_setup, ai_client_teardown), cmocka_unit_test_setup_teardown(test_ai_set_provider_setting, ai_client_setup, ai_client_teardown), cmocka_unit_test_setup_teardown(test_ai_set_provider_setting_reserved_key, ai_client_setup, ai_client_teardown), + cmocka_unit_test_setup_teardown(test_ai_set_provider_api_type, ai_client_setup, ai_client_teardown), + cmocka_unit_test(test_ai_api_type_string_round_trip), cmocka_unit_test_setup_teardown(test_ai_get_provider_setting, ai_client_setup, ai_client_teardown), /* Model caching tests */ cmocka_unit_test_setup_teardown(test_ai_models_are_fresh_initial, ai_client_setup, ai_client_teardown), @@ -775,8 +777,17 @@ main(int argc, char* argv[]) cmocka_unit_test(test_ai_json_escape_backslash_quote), /* Chat response parser */ cmocka_unit_test(test_ai_parse_response_openai_content), - cmocka_unit_test(test_ai_parse_response_legacy_text_format_unsupported), + cmocka_unit_test(test_ai_parse_response_responses_output_text), + cmocka_unit_test(test_ai_parse_response_responses_full_envelope), + cmocka_unit_test(test_ai_parse_response_responses_reasoning_summary_skipped), + cmocka_unit_test(test_ai_parse_response_choices_win_over_output), + cmocka_unit_test(test_ai_parse_response_choices_in_string_value), + cmocka_unit_test(test_ai_parse_response_content_before_choices), cmocka_unit_test(test_ai_parse_response_content_preferred_over_text), + cmocka_unit_test(test_ai_parse_response_responses_brace_in_text), + cmocka_unit_test(test_ai_parse_response_choices_logprobs_before_content), + cmocka_unit_test(test_ai_parse_response_reasoning_not_leaked_without_text), + cmocka_unit_test(test_ai_parse_response_responses_text_value_text), cmocka_unit_test(test_ai_parse_response_escaped_quote), cmocka_unit_test(test_ai_parse_response_newline_escape), cmocka_unit_test(test_ai_parse_response_empty_content), @@ -831,6 +842,7 @@ main(int argc, char* argv[]) /* Prefs round-trip (uses prefs+ai setup) */ cmocka_unit_test_setup_teardown(test_ai_prefs_round_trip_api_key, ai_client_setup_with_prefs, ai_client_teardown_with_prefs), cmocka_unit_test_setup_teardown(test_ai_prefs_round_trip_remove_key, ai_client_setup_with_prefs, ai_client_teardown_with_prefs), + cmocka_unit_test_setup_teardown(test_ai_prefs_round_trip_api_type, ai_client_setup_with_prefs, ai_client_teardown_with_prefs), cmocka_unit_test_setup_teardown(test_ai_prefs_multiple_providers_persist, ai_client_setup_with_prefs, ai_client_teardown_with_prefs), /* Autocomplete deeper coverage */ cmocka_unit_test_setup_teardown(test_ai_providers_find_after_remove_skips_removed, ai_client_setup, ai_client_teardown), -- 2.49.1 From b3a269342d82e5a38f25fd66f76c07c0602d1fe5 Mon Sep 17 00:00:00 2001 From: "jabber.developer2" Date: Mon, 6 Jul 2026 11:08:58 +0300 Subject: [PATCH 2/4] fix(ai): repair custom setting name autocompletion The setting-name branch for /ai set custom never fired: it passed the prefix "/ai set custom " with a trailing space to autocomplete_param_with_ac(), which appends a space itself, so the matched prefix contained a double space no real input ever has. Even without that, the primitive completes the first token after the command while the setting name is the second, and the num_args guard only held before the name was started. The suggestion list (tools/search/memory/ plugins) predated the /ai set custom backend and matched no real payload parameter. Replace the branch with token-position completion (arg 5 via autocomplete_param_no_with_func) fed by a list rebuilt on demand from common payload parameters plus the keys already set on the provider, exposed through the new ai_get_provider_setting_keys() (snapshot taken under settings_lock). Drop the now-unused parse_args() call whose NULL result was fed to g_strv_length() unguarded, spamming a glib CRITICAL on TAB at /ai with zero or five-plus arguments. --- src/ai/ai_client.c | 19 ++++++++ src/ai/ai_client.h | 8 ++++ src/command/cmd_ac.c | 77 +++++++++++++++++++++++--------- tests/unittests/test_ai_client.c | 28 ++++++++++++ tests/unittests/test_ai_client.h | 1 + tests/unittests/unittests.c | 1 + 6 files changed, 112 insertions(+), 22 deletions(-) diff --git a/src/ai/ai_client.c b/src/ai/ai_client.c index 8bd48767..221f4e30 100644 --- a/src/ai/ai_client.c +++ b/src/ai/ai_client.c @@ -990,6 +990,25 @@ ai_get_provider_setting(const gchar* provider_name, const gchar* setting) return value; } +GList* +ai_get_provider_setting_keys(const gchar* provider_name) +{ + AIProvider* provider = provider_name ? ai_get_provider(provider_name) : NULL; + if (!provider || !provider->settings) + return NULL; + + GList* keys = NULL; + pthread_mutex_lock(&provider->settings_lock); + GHashTableIter iter; + gpointer key, value; + g_hash_table_iter_init(&iter, provider->settings); + while (g_hash_table_iter_next(&iter, &key, &value)) { + keys = g_list_prepend(keys, g_strdup((gchar*)key)); + } + pthread_mutex_unlock(&provider->settings_lock); + return keys; +} + /* ======================================================================== * Generic HTTP request handling * ======================================================================== */ diff --git a/src/ai/ai_client.h b/src/ai/ai_client.h index f29dddd5..e8fa5e73 100644 --- a/src/ai/ai_client.h +++ b/src/ai/ai_client.h @@ -207,6 +207,14 @@ gboolean ai_set_provider_setting(const gchar* provider_name, const gchar* settin */ gchar* ai_get_provider_setting(const gchar* provider_name, const gchar* setting); +/** + * List the custom setting keys currently set on a provider. + * @param provider_name The provider name + * @return GList of gchar* (free with g_list_free_full(keys, g_free)), + * or NULL if the provider is unknown or has no settings + */ +GList* ai_get_provider_setting_keys(const gchar* provider_name); + /** * Fetch available models from a provider's API. * @param provider_name The provider name diff --git a/src/command/cmd_ac.c b/src/command/cmd_ac.c index 3bcb2022..4eae90dd 100644 --- a/src/command/cmd_ac.c +++ b/src/command/cmd_ac.c @@ -1192,11 +1192,6 @@ cmd_ac_init(void) autocomplete_add_unsorted(ai_api_types_ac, "responses", FALSE); autocomplete_add_unsorted(ai_api_types_ac, "auto", FALSE); - autocomplete_add_unsorted(ai_set_custom_subcommands_ac, "tools", FALSE); - autocomplete_add_unsorted(ai_set_custom_subcommands_ac, "search", FALSE); - autocomplete_add_unsorted(ai_set_custom_subcommands_ac, "memory", FALSE); - autocomplete_add_unsorted(ai_set_custom_subcommands_ac, "plugins", FALSE); - autocomplete_add_unsorted(ai_remove_subcommands_ac, "provider", FALSE); autocomplete_add(avatar_ac, "set"); @@ -4545,17 +4540,61 @@ _ai_api_type_find(const char* const search_str, gboolean previous, void* context return autocomplete_complete(ai_api_types_ac, search_str, FALSE, previous); } +static char* +_ai_custom_setting_find(const char* const search_str, gboolean previous, void* context) +{ + return autocomplete_complete(ai_set_custom_subcommands_ac, search_str, FALSE, previous); +} + +/** + * Autocomplete the setting name for /ai set custom . + * Offers a few common payload parameters merged with the keys already set + * on the provider, so existing settings can be retyped or overridden. + */ +static char* +_ai_custom_setting_autocomplete(const char* const input, gboolean previous) +{ + if (!g_str_has_prefix(input, "/ai set custom ")) { + return NULL; + } + + /* Quote-aware parse: args[2] is the provider, args[3] the setting being + * typed; skip the rebuild when the caret is not at the setting position */ + gboolean parse_result = FALSE; + auto_gcharv gchar** args = parse_args(input, 1, 5, &parse_result); + if (!parse_result) { + return NULL; + } + guint num_args = g_strv_length(args); + if (num_args < 3 || num_args > 4 || !ai_get_provider(args[2])) { + return NULL; + } + + /* Rebuild the completion list: common parameters + the provider's keys */ + gchar* suggestions[] = { "temperature", "max_tokens", "top_p" }; + GList* keys = ai_get_provider_setting_keys(args[2]); + char** items = g_new0(char*, ARRAY_SIZE(suggestions) + g_list_length(keys) + 1); + guint i = 0; + for (; i < ARRAY_SIZE(suggestions); i++) { + items[i] = suggestions[i]; + } + for (GList* curr = keys; curr; curr = g_list_next(curr)) { + items[i++] = curr->data; + } + autocomplete_update(ai_set_custom_subcommands_ac, items); + g_free(items); + g_list_free_full(keys, g_free); + + /* /ai(1) set(2) custom(3) (4) (5) */ + return autocomplete_param_no_with_func(input, "/ai set custom", 5, _ai_custom_setting_find, previous, NULL); +} + static char* _ai_autocomplete(ProfWin* window, const char* const input, gboolean previous) { char* result = NULL; - /* Parse once for reuse - /ai [] [] */ - gboolean parse_result = FALSE; - auto_gcharv gchar** args = parse_args(input, 1, 4, &parse_result); - int num_args = g_strv_length(args); - - /* Top-level /ai autocomplete (e.g., /ai s -> /ai set) */ + // /ai set provider - autocomplete provider names (for updating) result = autocomplete_param_with_func(input, "/ai set provider", ai_providers_find, previous, NULL); if (result) { return result; @@ -4591,18 +4630,12 @@ _ai_autocomplete(ProfWin* window, const char* const input, gboolean previous) return result; } - // /ai set custom - autocomplete settings - /* args[0]="set", args[1]="custom", args[2]=provider (if typed), args[3]=setting (if typed) */ - if (num_args == 3 && g_strcmp0(args[1], "custom") == 0) { - /* /ai set custom - check if provider is valid */ - if (ai_get_provider(args[2])) { - /* Valid provider, try settings autocomplete */ - result = autocomplete_param_with_ac(input, "/ai set custom ", ai_set_custom_subcommands_ac, TRUE, previous); - if (result) { - return result; - } - } + // /ai set custom - autocomplete setting names + result = _ai_custom_setting_autocomplete(input, previous); + if (result) { + return result; } + // /ai set - autocomplete subcommands result = autocomplete_param_with_ac(input, "/ai set", ai_set_subcommands_ac, TRUE, previous); if (result) { diff --git a/tests/unittests/test_ai_client.c b/tests/unittests/test_ai_client.c index aa6205b4..708447eb 100644 --- a/tests/unittests/test_ai_client.c +++ b/tests/unittests/test_ai_client.c @@ -637,6 +637,34 @@ test_ai_set_provider_setting_reserved_key(void** state) assert_string_equal("0.7", temp); } +void +test_ai_get_provider_setting_keys(void** state) +{ + ai_add_provider("p_setting_keys", "https://example.test/skeys/"); + + /* No settings yet, unknown provider, NULL — all yield NULL */ + assert_null(ai_get_provider_setting_keys("p_setting_keys")); + assert_null(ai_get_provider_setting_keys("nonexistent")); + assert_null(ai_get_provider_setting_keys(NULL)); + + ai_set_provider_setting("p_setting_keys", "temperature", "0.7"); + ai_set_provider_setting("p_setting_keys", "reasoning_effort", "high"); + + /* Both keys listed (order is hash-defined, assert membership) */ + GList* keys = ai_get_provider_setting_keys("p_setting_keys"); + assert_int_equal(2, g_list_length(keys)); + assert_non_null(g_list_find_custom(keys, "temperature", (GCompareFunc)g_strcmp0)); + assert_non_null(g_list_find_custom(keys, "reasoning_effort", (GCompareFunc)g_strcmp0)); + g_list_free_full(keys, g_free); + + /* Removing a setting removes its key */ + ai_set_provider_setting("p_setting_keys", "temperature", NULL); + keys = ai_get_provider_setting_keys("p_setting_keys"); + assert_int_equal(1, g_list_length(keys)); + assert_string_equal("reasoning_effort", keys->data); + g_list_free_full(keys, g_free); +} + void test_ai_set_provider_api_type(void** state) { diff --git a/tests/unittests/test_ai_client.h b/tests/unittests/test_ai_client.h index 6b274475..f35b007d 100644 --- a/tests/unittests/test_ai_client.h +++ b/tests/unittests/test_ai_client.h @@ -44,6 +44,7 @@ void test_ai_set_provider_default_model(void** state); void test_ai_get_provider_default_model(void** state); void test_ai_set_provider_setting(void** state); void test_ai_set_provider_setting_reserved_key(void** state); +void test_ai_get_provider_setting_keys(void** state); void test_ai_set_provider_api_type(void** state); void test_ai_api_type_string_round_trip(void** state); void test_ai_get_provider_setting(void** state); diff --git a/tests/unittests/unittests.c b/tests/unittests/unittests.c index 523a2321..94ff7965 100644 --- a/tests/unittests/unittests.c +++ b/tests/unittests/unittests.c @@ -753,6 +753,7 @@ main(int argc, char* argv[]) cmocka_unit_test_setup_teardown(test_ai_get_provider_default_model, ai_client_setup, ai_client_teardown), cmocka_unit_test_setup_teardown(test_ai_set_provider_setting, ai_client_setup, ai_client_teardown), cmocka_unit_test_setup_teardown(test_ai_set_provider_setting_reserved_key, ai_client_setup, ai_client_teardown), + cmocka_unit_test_setup_teardown(test_ai_get_provider_setting_keys, ai_client_setup, ai_client_teardown), cmocka_unit_test_setup_teardown(test_ai_set_provider_api_type, ai_client_setup, ai_client_teardown), cmocka_unit_test(test_ai_api_type_string_round_trip), cmocka_unit_test_setup_teardown(test_ai_get_provider_setting, ai_client_setup, ai_client_teardown), -- 2.49.1 From 3a96d0dba1630e9bf98dfc47703de88ade10b2a6 Mon Sep 17 00:00:00 2001 From: "jabber.developer2" Date: Wed, 8 Jul 2026 20:31:19 +0300 Subject: [PATCH 3/4] fix(ai): close parser and AUTO fallback gaps Parser: - Never return reasoning text as the reply: the no-output_text path anchors strictly on the message "content" array; a reasoning-only body (truncated by max_output_tokens, carries only "summary") now fails the parse instead of leaking the chain-of-thought. Regression test added. - _find_json_field skips field-name occurrences that are string values (no ':' after), so {"type":"text","text":"hi"} parses instead of failing on the value of "type". - _same_json_object tracks brace depth, so a nested object between the "text" key and the "output_text" tag (e.g. "annotations") no longer rejects the part's own text. - Chat-completions extraction is bounded to the choices array (new _json_array_end helper), so "message"/"content" in a sibling object (e.g. a top-level "warning") is not returned as the reply. AUTO fallback: - Model errors are classified only from a structured JSON error envelope; a route-missing 404 page merely mentioning models (e.g. listing /v1/models) no longer suppresses the fallback. Trade-off: a model error in a non-JSON body costs one extra fallback attempt. - Fall back on a curl-level failure: an endpoint that accepts the connection but never responds no longer burns the 60s timeout without trying the flavour that works. - Fall back on an unparseable 2xx: a gateway answering unknown paths with a 200 error page previously reset the hint and re-probed the same dead flavour on every request. The body is parsed inside the attempt loop and no longer re-parsed after it. - The second attempt's 401/403/429/5xx now wins over the stashed 400/422 payload rejection, so an invalid key or rate limit is not hidden behind a payload-shape error; a second 404 still defers to the first. - When the first flavour's endpoint is missing but the other answers (even unsuccessfully, e.g. 401), the resolved hint moves to the surviving flavour, so the dead endpoint is not probed first forever. Settings: - Reserved payload keys are rejected only when setting a value; removal is allowed again, so a setting persisted before its key became reserved (e.g. "input") is no longer stuck in the config. --- src/ai/ai_client.c | 185 +++++++++++++++++++++++-------- tests/unittests/test_ai_client.c | 13 +++ tests/unittests/test_ai_client.h | 1 + tests/unittests/unittests.c | 1 + 4 files changed, 151 insertions(+), 49 deletions(-) diff --git a/src/ai/ai_client.c b/src/ai/ai_client.c index 221f4e30..029aa00d 100644 --- a/src/ai/ai_client.c +++ b/src/ai/ai_client.c @@ -37,6 +37,7 @@ static void _ai_load_models_for_provider(AIProvider* provider); static void _ai_save_models_for_provider(AIProvider* provider); static const gchar* _find_json_field(const gchar* json, const gchar* field); static gchar* _extract_json_string(const gchar* json, const gchar* field); +static const gchar* _json_array_end(const gchar* arr); /* ======================================================================== * Curl helpers @@ -473,15 +474,16 @@ _body_names_model_error(const gchar* body) if (!body) return FALSE; + /* Only trust a structured error envelope. Matching the raw body also + * fires on route-missing 404 pages that merely mention models (e.g. an + * HTML 404 listing /v1/models), which would wrongly suppress the fallback */ const gchar* err = strstr(body, "\"error\""); - if (err) { - auto_gchar gchar* code = _extract_json_string(err, "code"); - auto_gchar gchar* type = _extract_json_string(err, "type"); - auto_gchar gchar* message = _extract_json_string(err, "message"); - if (code || type || message) - return _names_model(code) || _names_model(type) || _names_model(message); - } - return _names_model(body); + if (!err) + return FALSE; + auto_gchar gchar* code = _extract_json_string(err, "code"); + auto_gchar gchar* type = _extract_json_string(err, "type"); + auto_gchar gchar* message = _extract_json_string(err, "message"); + return _names_model(code) || _names_model(type) || _names_model(message); } static gboolean @@ -940,7 +942,9 @@ ai_set_provider_setting(const gchar* provider_name, const gchar* setting, const if (!provider_name || !setting) return FALSE; - if (_is_reserved_payload_key(setting)) { + /* Reject setting a reserved key, but still allow removing one so a stale + * entry persisted before the key was reserved can be cleared */ + if (value && _is_reserved_payload_key(setting)) { log_warning("Setting '%s' for provider '%s' rejected: reserved payload key", setting, provider_name); return FALSE; } @@ -1253,30 +1257,27 @@ _find_json_field(const gchar* json, const gchar* field) } auto_gchar gchar* key = g_strdup_printf("\"%s\"", field); - const gchar* pos = strstr(json, key); - if (!pos) { - return NULL; + gsize keylen = strlen(key); + + /* An occurrence not followed by ':' is a string value equal to the field + * name (e.g. "text" as the value of "type"), not the key — skip it and + * look for the next one instead of giving up */ + for (const gchar* search = json; (search = strstr(search, key)) != NULL; search += keylen) { + const gchar* pos = search + keylen; + while (*pos == ' ' || *pos == '\t' || *pos == '\n' || *pos == '\r') { + pos++; + } + if (*pos != ':') { + continue; + } + pos++; /* skip colon */ + while (*pos == ' ' || *pos == '\t' || *pos == '\n' || *pos == '\r') { + pos++; + } + return pos; } - /* Skip past the key */ - pos += strlen(key); - - /* Skip whitespace and find the colon */ - while (*pos == ' ' || *pos == '\t' || *pos == '\n' || *pos == '\r') { - pos++; - } - - if (*pos != ':') { - return NULL; - } - pos++; /* skip colon */ - - /* Skip whitespace after colon */ - while (*pos == ' ' || *pos == '\t' || *pos == '\n' || *pos == '\r') { - pos++; - } - - return pos; + return NULL; } /** @@ -1791,16 +1792,28 @@ _parse_chat_completions(const gchar* json) const gchar* choices = strstr(json, "\"choices\""); if (!choices) return NULL; + /* Bound the search to the choices array so a "message"/"content" in a + * sibling object (e.g. a top-level "warning") is not taken as the reply */ + const gchar* arr = strchr(choices, '['); + const gchar* end = arr ? _json_array_end(arr) : NULL; const gchar* message = strstr(choices, "\"message\""); - return _extract_json_string(message ? message : choices, "content"); + if (message && end && message >= end) + message = NULL; + const gchar* anchor = message ? message : choices; + const gchar* cpos = _find_json_field(anchor, "content"); + if (!cpos || (end && cpos >= end)) + return NULL; + return _extract_json_string(anchor, "content"); } -/* TRUE when no object boundary ({ or }) lies between @from and @to outside of - * string literals — i.e. both positions belong to the same JSON object */ +/* TRUE when @from and @to belong to the same JSON object: a balanced nested + * sub-object between them is fine, but a brace that exits the enclosing object + * (net depth < 0) or leaves us inside a deeper one (net depth != 0) is not */ static gboolean _same_json_object(const gchar* from, const gchar* to) { gboolean in_string = FALSE; + gint depth = 0; for (const gchar* p = from; p < to && *p; p++) { if (in_string) { if (*p == '\\' && *(p + 1) != '\0') @@ -1809,11 +1822,40 @@ _same_json_object(const gchar* from, const gchar* to) in_string = FALSE; } else if (*p == '"') { in_string = TRUE; - } else if (*p == '{' || *p == '}') { - return FALSE; + } else if (*p == '{') { + depth++; + } else if (*p == '}') { + if (depth == 0) + return FALSE; + depth--; } } - return TRUE; + return depth == 0; +} + +/* Pointer just past the ']' matching the '[' at @arr, or NULL if unterminated. + * Brackets inside string literals are ignored. */ +static const gchar* +_json_array_end(const gchar* arr) +{ + gboolean in_string = FALSE; + gint depth = 0; + for (const gchar* p = arr; *p; p++) { + if (in_string) { + if (*p == '\\' && *(p + 1) != '\0') + p++; + else if (*p == '"') + in_string = FALSE; + } else if (*p == '"') { + in_string = TRUE; + } else if (*p == '[') { + depth++; + } else if (*p == ']') { + if (--depth == 0) + return p + 1; + } + } + return NULL; } /* Responses: {"output":[...,{"content":[{"type":"output_text","text":"..."}]}]}. @@ -1851,10 +1893,13 @@ _parse_responses(const gchar* json) } /* No output_text tag (truncated reply, or a server tagging the part - * "type":"text"): anchor on the message "content" array so a reasoning - * "summary" text is not returned as the reply */ + * "type":"text"): anchor strictly on the message "content" array. A + * reasoning-only body carries "summary", not "content", so returning NULL + * here surfaces a parse error instead of leaking the reasoning text */ const gchar* content_arr = strstr(output, "\"content\""); - return _extract_json_string(content_arr ? content_arr : output, "text"); + if (!content_arr) + return NULL; + return _extract_json_string(content_arr, "text"); } /* Extract assistant content, trying the known request flavour's extractor @@ -2021,6 +2066,8 @@ _ai_request_thread(gpointer data) * actionable error; keep it in case the other flavour also fails */ auto_gchar gchar* first_reject_body = NULL; long first_reject_code = 0; + auto_gchar gchar* content = NULL; + gboolean first_endpoint_missing = FALSE; for (gint i = 0; i < n_attempts; i++) { used_type = attempts[i]; auto_gchar gchar* request_url = g_strdup_printf("%s%s", base_url, _api_type_endpoint(used_type)); @@ -2043,7 +2090,13 @@ _ai_request_thread(gpointer data) curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &http_code); log_debug("[AI-THREAD] HTTP response code: %ld", http_code); - if (res == CURLE_OK && i + 1 < n_attempts && _should_try_other_flavour(http_code, response.data)) { + if (i == 0 && res == CURLE_OK && _is_endpoint_missing(http_code, response.data)) + first_endpoint_missing = TRUE; + + gboolean more = (i + 1 < n_attempts); + + /* Endpoint missing or payload-shape rejection: fall back to the other flavour */ + if (res == CURLE_OK && more && _should_try_other_flavour(http_code, response.data)) { log_info("[AI-THREAD] %s endpoint rejected request for '%s' (HTTP %ld), falling back to %s", ai_api_type_to_string(used_type), local_provider_name, http_code, ai_api_type_to_string(attempts[i + 1])); @@ -2055,13 +2108,44 @@ _ai_request_thread(gpointer data) } continue; } + + /* Connectivity failure (timeout/reset/DNS) on this flavour's endpoint: + * the other flavour may still be reachable, so try it before failing */ + if (res != CURLE_OK && more) { + log_info("[AI-THREAD] %s request for '%s' failed (%s), trying %s", + ai_api_type_to_string(used_type), local_provider_name, + curl_easy_strerror(res), ai_api_type_to_string(attempts[i + 1])); + continue; + } + + /* Parse a success body here so a 2xx we cannot parse during an AUTO + * probe (e.g. a proxy 200 page) falls back to the other flavour + * instead of wedging on this endpoint */ + if (res == CURLE_OK && http_code < 400) { + g_free(content); + content = ai_parse_response_typed(used_type, response.data); + if (!content && more && configured == AI_API_TYPE_AUTO + && http_code >= 200 && http_code < 300) { + log_info("[AI-THREAD] %s returned an unparseable 2xx for '%s', trying %s", + ai_api_type_to_string(used_type), local_provider_name, + ai_api_type_to_string(attempts[i + 1])); + continue; + } + } break; } auto_gchar gchar* response_data = g_steal_pointer(&response.data); - if (configured == AI_API_TYPE_AUTO && res == CURLE_OK && _is_endpoint_missing(http_code, response_data)) { - /* The flavour we tried is no longer served; re-probe on the next request */ - _provider_set_resolved(local_provider, AI_API_TYPE_AUTO, api_epoch); + if (configured == AI_API_TYPE_AUTO && res == CURLE_OK) { + if (_is_endpoint_missing(http_code, response_data)) { + /* The flavour we ended on is not served; re-probe on the next request */ + _provider_set_resolved(local_provider, AI_API_TYPE_AUTO, api_epoch); + } else if (first_endpoint_missing) { + /* The first (hint/default) flavour's endpoint is gone but this one + * answered; prefer it as the hint so later requests stop probing + * the dead endpoint, even if this request itself failed (e.g. auth) */ + _provider_set_resolved(local_provider, used_type, api_epoch); + } } if (res != CURLE_OK) { @@ -2069,10 +2153,14 @@ _ai_request_thread(gpointer data) log_error("AI request failed for %s/%s: %s", local_provider_name, local_model, error_msg); _aiwin_display_error(user_data, error_msg); } else if (http_code >= 400) { - /* Handle HTTP errors; a payload rejection that triggered the fallback - * is more actionable than the other endpoint's follow-up failure */ - const gchar* err_body = first_reject_body ? first_reject_body : response_data; - long err_code = first_reject_body ? first_reject_code : http_code; + /* The first attempt's 400/422 is usually the actionable error, but an + * auth/rate-limit/server failure on the second attempt (401/403/429/5xx) + * is more actionable — surface that rather than the stashed payload error */ + gboolean second_more_actionable = http_code == 401 || http_code == 403 + || http_code == 429 || http_code >= 500; + gboolean use_first = first_reject_body && !second_more_actionable; + const gchar* err_body = use_first ? first_reject_body : response_data; + long err_code = use_first ? first_reject_code : http_code; log_debug("[AI-THREAD] HTTP error response body (%zu bytes): %s", response.size, response_data); /* Try to extract the actual error message from the JSON response */ @@ -2082,7 +2170,6 @@ _ai_request_thread(gpointer data) _aiwin_display_error(user_data, error_msg); } else { log_debug("[AI-THREAD] Raw API response (%zu bytes): %s", response.size, response_data); - auto_gchar gchar* content = ai_parse_response_typed(used_type, response_data); if (content) { if (configured == AI_API_TYPE_AUTO && http_code >= 200 && http_code < 300) { /* Remember the flavour that worked so later requests try it diff --git a/tests/unittests/test_ai_client.c b/tests/unittests/test_ai_client.c index 708447eb..ccff7363 100644 --- a/tests/unittests/test_ai_client.c +++ b/tests/unittests/test_ai_client.c @@ -1074,6 +1074,19 @@ test_ai_parse_response_responses_reasoning_summary_skipped(void** state) assert_string_equal("Actual answer", out); } +void +test_ai_parse_response_responses_reasoning_only_returns_null(void** state) +{ + /* An incomplete/truncated body with only a reasoning item (no message + * "content" array at all) must yield a parse failure, not leak the + * reasoning summary's "text" as the assistant reply. */ + const gchar* json = "{\"status\":\"incomplete\",\"output\":[" + "{\"type\":\"reasoning\",\"summary\":[{\"type\":\"summary_text\",\"text\":\"secret reasoning\"}]}" + "]}"; + gchar* out = ai_parse_response(json); + assert_null(out); +} + void test_ai_parse_response_choices_in_string_value(void** state) { diff --git a/tests/unittests/test_ai_client.h b/tests/unittests/test_ai_client.h index f35b007d..03a4ff91 100644 --- a/tests/unittests/test_ai_client.h +++ b/tests/unittests/test_ai_client.h @@ -78,6 +78,7 @@ void test_ai_parse_response_content_preferred_over_text(void** state); void test_ai_parse_response_responses_brace_in_text(void** state); void test_ai_parse_response_choices_logprobs_before_content(void** state); void test_ai_parse_response_reasoning_not_leaked_without_text(void** state); +void test_ai_parse_response_responses_reasoning_only_returns_null(void** state); void test_ai_parse_response_responses_text_value_text(void** state); void test_ai_parse_response_escaped_quote(void** state); void test_ai_parse_response_newline_escape(void** state); diff --git a/tests/unittests/unittests.c b/tests/unittests/unittests.c index 94ff7965..4907b8a0 100644 --- a/tests/unittests/unittests.c +++ b/tests/unittests/unittests.c @@ -788,6 +788,7 @@ main(int argc, char* argv[]) cmocka_unit_test(test_ai_parse_response_responses_brace_in_text), cmocka_unit_test(test_ai_parse_response_choices_logprobs_before_content), cmocka_unit_test(test_ai_parse_response_reasoning_not_leaked_without_text), + cmocka_unit_test(test_ai_parse_response_responses_reasoning_only_returns_null), cmocka_unit_test(test_ai_parse_response_responses_text_value_text), cmocka_unit_test(test_ai_parse_response_escaped_quote), cmocka_unit_test(test_ai_parse_response_newline_escape), -- 2.49.1 From 37218d2e075048882cbae9cb3327c6ef827a57c5 Mon Sep 17 00:00:00 2001 From: "jabber.developer2" Date: Fri, 10 Jul 2026 11:36:45 +0300 Subject: [PATCH 4/4] fix(ai): bound response parsing and harden AUTO fallback errors - bound _parse_responses scans to the output_text part's object and to the content array, so a later sibling item's "text" (e.g. a reasoning summary) can never be returned as the assistant reply - do not retry the other flavour on a curl timeout: the request likely reached the server and may still be generating, so a re-POST of the conversation could trigger a second billed generation - remember an unparseable 2xx from the first AUTO attempt and surface stashed first-attempt errors in both error paths, instead of showing only the final attempt's transport or HTTP error - recognize Ollama's model-not-found wording in _names_model so a model typo is not misread as a missing endpoint - match model errors case-insensitively and recognize the 'model "x" is not supported' wording, so a wrong-model 400 (seen live on Perplexity /v1/responses) fails fast instead of probing the other flavour with a doomed request - say "empty response body" instead of "Unknown error" when an HTTP error arrives without a body (e.g. Perplexity's bare 404) - drop the dead, racy provider-lookup fallback in the generic request thread: a missing provider ref is a caller bug and now fails loudly - fix ai_providers_lists_defaults to expect the header the command actually prints ("Configured providers:"); the test was broken since its introduction but CI never ran it - add functional test group 5 (AI command surface) to FUNC_TEST_GROUPS so the CI parallel target runs it; proftest.c port ranges already account for five groups --- Makefile.am | 2 +- src/ai/ai_client.c | 150 +++++++++++++++++++++++-------- tests/functionaltests/test_ai.c | 4 +- tests/unittests/test_ai_client.c | 42 +++++++++ tests/unittests/test_ai_client.h | 3 + tests/unittests/unittests.c | 3 + 6 files changed, 166 insertions(+), 38 deletions(-) diff --git a/Makefile.am b/Makefile.am index c3e3503f..e44b38af 100644 --- a/Makefile.am +++ b/Makefile.am @@ -335,7 +335,7 @@ tests_functionaltests_functionaltests_LDADD = -lcmocka -lstabber @FORKPTY_LIB@ # Parallel functional tests target (~3x faster than sequential) # Usage: make check-functional-parallel # To add more groups: increase FUNC_TEST_GROUPS and add group in functionaltests.c -FUNC_TEST_GROUPS = 1 2 3 4 +FUNC_TEST_GROUPS = 1 2 3 4 5 check-functional-parallel: tests/functionaltests/functionaltests @echo "Running functional tests in parallel ($(words $(FUNC_TEST_GROUPS)) groups)..." diff --git a/src/ai/ai_client.c b/src/ai/ai_client.c index 029aa00d..d0d8e7c8 100644 --- a/src/ai/ai_client.c +++ b/src/ai/ai_client.c @@ -380,14 +380,12 @@ _json_unescape_substring(const gchar* start, const gchar* end) return g_realloc(out, w - out + 1); } -/* Locate "field":"...value..." in json and return the unescaped value. - * Skips JSON whitespace around the colon and treats any \X (X != \0) inside - * the string body as an opaque escape so an embedded \" cannot be misread as - * the closing quote. */ +/* Unescape the string value starting at @val (a pointer to its opening quote). + * Treats any \X (X != \0) inside the string body as an opaque escape so an + * embedded \" cannot be misread as the closing quote. */ static gchar* -_extract_json_string(const gchar* json, const gchar* field) +_extract_json_string_at(const gchar* val) { - const gchar* val = _find_json_field(json, field); if (!val || *val != '"') return NULL; @@ -404,6 +402,24 @@ _extract_json_string(const gchar* json, const gchar* field) return NULL; } +/* Locate "field":"...value..." in json and return the unescaped value. + * Skips JSON whitespace around the colon. @end (optional) bounds the search: + * a value starting at or past it is not extracted. */ +static gchar* +_extract_json_string_within(const gchar* json, const gchar* field, const gchar* end) +{ + const gchar* val = _find_json_field(json, field); + if (!val || (end && val >= end)) + return NULL; + return _extract_json_string_at(val); +} + +static gchar* +_extract_json_string(const gchar* json, const gchar* field) +{ + return _extract_json_string_within(json, field, NULL); +} + /* ======================================================================== * API type helpers * ======================================================================== */ @@ -456,11 +472,16 @@ _names_model(const gchar* s) { if (!s) return FALSE; - /* "does not exist" alone also appears in route-missing bodies; require it - * to talk about a model */ - return strstr(s, "model_not_found") || strstr(s, "invalid_model") - || strstr(s, "unknown_model") || strstr(s, "model not found") - || (strstr(s, "model") && strstr(s, "does not exist")); + /* "does not exist"/"not found"/"not supported" alone also appear in + * route-missing bodies; require them to talk about a model (covers + * Ollama's 'model "x" not found, try pulling it first' and Perplexity's + * 'model "x" is not supported'). Gateways vary in casing, so compare + * case-insensitively. */ + auto_gchar gchar* lower = g_ascii_strdown(s, -1); + return strstr(lower, "model_not_found") || strstr(lower, "invalid_model") + || strstr(lower, "unknown_model") + || (strstr(lower, "model") + && (strstr(lower, "does not exist") || strstr(lower, "not found") || strstr(lower, "not supported"))); } /* A missing endpoint answers 404/405/501. A wrong model can also 404/405, but @@ -1022,7 +1043,7 @@ typedef void (*ai_response_handler_t)(AIProvider* provider, const gchar* respons typedef struct { gchar* provider_name; - AIProvider* provider; /* owned ref taken on the main thread, or NULL */ + AIProvider* provider; /* owned ref taken on the main thread; required */ gpointer user_data; gchar* request_url; ai_response_handler_t handler; @@ -1071,7 +1092,7 @@ _curl_exec_and_handle(CURL* curl, const gchar* url, struct curl_slist* headers, auto_gchar gchar* error_msg = parsed ? g_strdup_printf("HTTP %ld: %s", http_code, parsed) : g_strdup_printf("HTTP %ld: %s", http_code, - response->data && strlen(response->data) > 0 ? response->data : "Unknown error"); + response->data && strlen(response->data) > 0 ? response->data : "empty response body"); log_error("Request HTTP error for '%s': %s", provider->name, error_msg); _aiwin_display_error(user_data, error_msg); } else { @@ -1086,20 +1107,16 @@ _ai_generic_request_thread(gpointer data) { ai_request_t* req = (ai_request_t*)data; - /* Keep the provider alive for the duration of this request. Prefer the ref - * taken on the main thread (guaranteed pre-removal); the lookup fallback - * still has a small window against /ai remove provider. */ + /* The ref taken on the main thread keeps the provider alive for the + * duration of this request; a thread-side lookup instead would race + * against /ai remove provider, so its absence is a caller bug */ AIProvider* provider = g_steal_pointer(&req->provider); if (!provider) { - provider = ai_get_provider(req->provider_name); - if (!provider) { - log_error("Provider '%s' not found for request", req->provider_name); - g_free(req->provider_name); - g_free(req->request_url); - g_free(req); - return NULL; - } - ai_provider_ref(provider); + log_error("BUG: request for '%s' spawned without a provider ref", req->provider_name); + g_free(req->provider_name); + g_free(req->request_url); + g_free(req); + return NULL; } CURL* curl = curl_easy_init(); @@ -1135,7 +1152,7 @@ _ai_generic_request_thread(gpointer data) curl_easy_cleanup(curl); g_free(response.data); - /* Release the reference taken after lookup — provider may now be freed. */ + /* Release the main-thread ref — provider may now be freed. */ ai_provider_unref(provider); g_free(req->provider_name); @@ -1833,6 +1850,33 @@ _same_json_object(const gchar* from, const gchar* to) return depth == 0; } +/* Pointer to the '}' closing the object that encloses @pos (which must sit + * outside a string literal, e.g. at a key's opening quote), or NULL if + * unterminated. Braces inside string literals are ignored. */ +static const gchar* +_json_object_end(const gchar* pos) +{ + gboolean in_string = FALSE; + gint depth = 0; + for (const gchar* p = pos; *p; p++) { + if (in_string) { + if (*p == '\\' && *(p + 1) != '\0') + p++; + else if (*p == '"') + in_string = FALSE; + } else if (*p == '"') { + in_string = TRUE; + } else if (*p == '{') { + depth++; + } else if (*p == '}') { + if (depth == 0) + return p; + depth--; + } + } + return NULL; +} + /* Pointer just past the ']' matching the '[' at @arr, or NULL if unterminated. * Brackets inside string literals are ignored. */ static const gchar* @@ -1870,7 +1914,11 @@ _parse_responses(const gchar* json) const gchar* part = strstr(output, "\"output_text\""); if (part) { - gchar* text = _extract_json_string(part, "text"); + /* Bound the forward scan to the part's own object so a "text" from a + * later sibling item (e.g. a following reasoning summary) can never + * be returned as the reply */ + const gchar* part_end = _json_object_end(part); + gchar* text = _extract_json_string_within(part, "text", part_end); if (text) return text; /* "text" may precede "type":"output_text" in the part: take the @@ -1883,7 +1931,7 @@ _parse_responses(const gchar* json) if (!key) break; if (_same_json_object(key, part)) { - text = _extract_json_string(key, "text"); + text = _extract_json_string_within(key, "text", part_end); if (text) return text; } @@ -1899,7 +1947,12 @@ _parse_responses(const gchar* json) const gchar* content_arr = strstr(output, "\"content\""); if (!content_arr) return NULL; - return _extract_json_string(content_arr, "text"); + /* Bound the scan to the content array itself: an empty or text-less array + * must fail the parse rather than pick up a "text" from a later item */ + const gchar* cval = _find_json_field(content_arr, "content"); + if (!cval || *cval != '[') + return NULL; + return _extract_json_string_within(cval, "text", _json_array_end(cval)); } /* Extract assistant content, trying the known request flavour's extractor @@ -2066,6 +2119,9 @@ _ai_request_thread(gpointer data) * actionable error; keep it in case the other flavour also fails */ auto_gchar gchar* first_reject_body = NULL; long first_reject_code = 0; + /* Same for a 2xx we could not parse: the user must learn their endpoint + * answered, even when the fallback attempt fails with its own error */ + auto_gchar gchar* first_unparsable_note = NULL; auto_gchar gchar* content = NULL; gboolean first_endpoint_missing = FALSE; for (gint i = 0; i < n_attempts; i++) { @@ -2109,9 +2165,12 @@ _ai_request_thread(gpointer data) continue; } - /* Connectivity failure (timeout/reset/DNS) on this flavour's endpoint: - * the other flavour may still be reachable, so try it before failing */ - if (res != CURLE_OK && more) { + /* Connectivity failure (reset/DNS/refused) on this flavour's endpoint: + * the other flavour may still be reachable, so try it before failing. + * A timeout is excluded: the request likely reached the server and may + * still be generating (and billing) — re-POSTing the conversation to + * the other endpoint could trigger a second generation */ + if (res != CURLE_OK && res != CURLE_OPERATION_TIMEDOUT && more) { log_info("[AI-THREAD] %s request for '%s' failed (%s), trying %s", ai_api_type_to_string(used_type), local_provider_name, curl_easy_strerror(res), ai_api_type_to_string(attempts[i + 1])); @@ -2126,9 +2185,12 @@ _ai_request_thread(gpointer data) content = ai_parse_response_typed(used_type, response.data); if (!content && more && configured == AI_API_TYPE_AUTO && http_code >= 200 && http_code < 300) { - log_info("[AI-THREAD] %s returned an unparseable 2xx for '%s', trying %s", + log_info("[AI-THREAD] %s returned an unparsable 2xx for '%s', trying %s", ai_api_type_to_string(used_type), local_provider_name, ai_api_type_to_string(attempts[i + 1])); + if (!first_unparsable_note) + first_unparsable_note = g_strdup_printf("%s endpoint answered HTTP %ld but the body could not be parsed", + ai_api_type_to_string(used_type), http_code); continue; } } @@ -2149,7 +2211,20 @@ _ai_request_thread(gpointer data) } if (res != CURLE_OK) { - auto_gchar gchar* error_msg = g_strdup(curl_easy_strerror(res)); + /* The stashed first-attempt failure is usually the actionable part: + * that endpoint did answer, this one did not */ + auto_gchar gchar* first_note = NULL; + if (first_reject_body) { + auto_gchar gchar* parsed = ai_parse_error_message(first_reject_body); + first_note = g_strdup_printf("%s endpoint rejected the request with HTTP %ld: %s", + ai_api_type_to_string(attempts[0]), first_reject_code, + parsed ? parsed : first_reject_body); + } else if (first_unparsable_note) { + first_note = g_strdup(first_unparsable_note); + } + auto_gchar gchar* error_msg = first_note + ? g_strdup_printf("%s (first attempt: %s)", curl_easy_strerror(res), first_note) + : g_strdup(curl_easy_strerror(res)); log_error("AI request failed for %s/%s: %s", local_provider_name, local_model, error_msg); _aiwin_display_error(user_data, error_msg); } else if (http_code >= 400) { @@ -2165,7 +2240,12 @@ _ai_request_thread(gpointer data) response.size, response_data); /* Try to extract the actual error message from the JSON response */ auto_gchar gchar* parsed_error = ai_parse_error_message(err_body); - auto_gchar gchar* error_msg = parsed_error ? g_strdup_printf("HTTP %ld: %s", err_code, parsed_error) : g_strdup_printf("HTTP %ld: %s", err_code, err_body && strlen(err_body) > 0 ? err_body : "Unknown error"); + auto_gchar gchar* error_msg = parsed_error ? g_strdup_printf("HTTP %ld: %s", err_code, parsed_error) : g_strdup_printf("HTTP %ld: %s", err_code, err_body && strlen(err_body) > 0 ? err_body : "empty response body"); + if (first_unparsable_note) { + gchar* with_note = g_strdup_printf("%s (first attempt: %s)", error_msg, first_unparsable_note); + g_free(error_msg); + error_msg = with_note; + } log_error("AI request failed for %s/%s: %s", local_provider_name, local_model, error_msg); _aiwin_display_error(user_data, error_msg); } else { diff --git a/tests/functionaltests/test_ai.c b/tests/functionaltests/test_ai.c index 9da93933..53742dae 100644 --- a/tests/functionaltests/test_ai.c +++ b/tests/functionaltests/test_ai.c @@ -55,11 +55,11 @@ ai_no_args_shows_help(void** state) void ai_providers_lists_defaults(void** state) { - /* `/ai providers` (no "list") shows the built-in list with URLs. */ + /* `/ai providers` (no "list") shows the configured list with URLs. */ prof_input("/ai providers"); prof_timeout(5); - assert_true(prof_output_exact("Available AI providers:")); + assert_true(prof_output_exact("Configured providers:")); /* At least one URL line is rendered — exact name agnostic. */ assert_true(prof_output_regex("https?://")); prof_timeout_reset(); diff --git a/tests/unittests/test_ai_client.c b/tests/unittests/test_ai_client.c index ccff7363..c8df36c3 100644 --- a/tests/unittests/test_ai_client.c +++ b/tests/unittests/test_ai_client.c @@ -1087,6 +1087,48 @@ test_ai_parse_response_responses_reasoning_only_returns_null(void** state) assert_null(out); } +void +test_ai_parse_response_responses_reasoning_after_message_skipped(void** state) +{ + /* A reasoning item ordered after the message, with the part's "text" + * preceding "type":"output_text": the forward scan must stay inside the + * part's object and the backscan must find the real answer. */ + const gchar* json = "{\"output\":[" + "{\"type\":\"message\",\"content\":[{\"text\":\"answer\",\"type\":\"output_text\"}]}," + "{\"type\":\"reasoning\",\"summary\":[{\"type\":\"summary_text\",\"text\":\"secret\"}]}" + "]}"; + auto_gchar gchar* out = ai_parse_response(json); + assert_non_null(out); + assert_string_equal("answer", out); +} + +void +test_ai_parse_response_responses_empty_content_not_leaked(void** state) +{ + /* An empty content array followed by a reasoning item must yield a parse + * failure; the scan must not escape the array and return the summary. */ + const gchar* json = "{\"output\":[" + "{\"type\":\"message\",\"content\":[]}," + "{\"type\":\"reasoning\",\"summary\":[{\"type\":\"summary_text\",\"text\":\"secret\"}]}" + "]}"; + gchar* out = ai_parse_response(json); + assert_null(out); +} + +void +test_ai_parse_response_responses_text_value_before_later_item(void** state) +{ + /* A reply that is exactly "text" plus a reasoning item after the message: + * the backscan's own extraction must also stay inside the part's object. */ + const gchar* json = "{\"output\":[" + "{\"type\":\"message\",\"content\":[{\"text\":\"text\",\"type\":\"output_text\"}]}," + "{\"type\":\"reasoning\",\"summary\":[{\"type\":\"summary_text\",\"text\":\"secret\"}]}" + "]}"; + auto_gchar gchar* out = ai_parse_response(json); + assert_non_null(out); + assert_string_equal("text", out); +} + void test_ai_parse_response_choices_in_string_value(void** state) { diff --git a/tests/unittests/test_ai_client.h b/tests/unittests/test_ai_client.h index 03a4ff91..b194c3b9 100644 --- a/tests/unittests/test_ai_client.h +++ b/tests/unittests/test_ai_client.h @@ -79,6 +79,9 @@ void test_ai_parse_response_responses_brace_in_text(void** state); void test_ai_parse_response_choices_logprobs_before_content(void** state); void test_ai_parse_response_reasoning_not_leaked_without_text(void** state); void test_ai_parse_response_responses_reasoning_only_returns_null(void** state); +void test_ai_parse_response_responses_reasoning_after_message_skipped(void** state); +void test_ai_parse_response_responses_empty_content_not_leaked(void** state); +void test_ai_parse_response_responses_text_value_before_later_item(void** state); void test_ai_parse_response_responses_text_value_text(void** state); void test_ai_parse_response_escaped_quote(void** state); void test_ai_parse_response_newline_escape(void** state); diff --git a/tests/unittests/unittests.c b/tests/unittests/unittests.c index 4907b8a0..1f2eaecc 100644 --- a/tests/unittests/unittests.c +++ b/tests/unittests/unittests.c @@ -789,6 +789,9 @@ main(int argc, char* argv[]) cmocka_unit_test(test_ai_parse_response_choices_logprobs_before_content), cmocka_unit_test(test_ai_parse_response_reasoning_not_leaked_without_text), cmocka_unit_test(test_ai_parse_response_responses_reasoning_only_returns_null), + cmocka_unit_test(test_ai_parse_response_responses_reasoning_after_message_skipped), + cmocka_unit_test(test_ai_parse_response_responses_empty_content_not_leaked), + cmocka_unit_test(test_ai_parse_response_responses_text_value_before_later_item), cmocka_unit_test(test_ai_parse_response_responses_text_value_text), cmocka_unit_test(test_ai_parse_response_escaped_quote), cmocka_unit_test(test_ai_parse_response_newline_escape), -- 2.49.1