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),