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.
This commit is contained in:
2026-07-08 20:31:19 +03:00
parent b3a269342d
commit 3a96d0dba1
4 changed files with 151 additions and 49 deletions

View File

@@ -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

View File

@@ -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)
{

View File

@@ -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);

View File

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