fix(ai): unbreak CI and harden custom settings after chat-completions alignment #152

Manually merged
jabber.developer2 merged 2 commits from fix/ai-chat-completions-followup into master 2026-07-04 11:21:29 +00:00
7 changed files with 121 additions and 47 deletions

View File

@@ -186,6 +186,22 @@ ai_json_escape(const gchar* str)
return g_string_free(result, FALSE);
}
/* Keys emitted as fixed payload fields; 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;
}
/* RFC 8259 scalars (number/true/false/null) may be emitted bare in a JSON payload */
static gboolean
_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);
}
/**
* Convert a 4-hex-digit sequence at @p to a 16-bit code point.
* Returns -1 on invalid hex.
@@ -392,6 +408,7 @@ ai_provider_new(const gchar* name, const gchar* api_url)
provider->models = NULL;
provider->models_fresh = FALSE;
provider->ref_count = 1;
pthread_mutex_init(&provider->settings_lock, NULL);
return provider;
}
@@ -419,6 +436,7 @@ ai_provider_unref(AIProvider* provider)
g_free(provider->project_id);
g_free(provider->default_model);
g_hash_table_destroy(provider->settings);
pthread_mutex_destroy(&provider->settings_lock);
GList* curr = provider->models;
while (curr) {
@@ -729,31 +747,44 @@ ai_get_provider_default_model(const gchar* provider_name)
return provider->default_model;
}
void
gboolean
ai_set_provider_setting(const gchar* provider_name, const gchar* setting, const gchar* value)
{
if (!provider_name || !setting)
return;
return FALSE;
if (_is_reserved_payload_key(setting)) {
log_warning("Setting '%s' for provider '%s' rejected: reserved payload key", setting, provider_name);
return FALSE;
}
AIProvider* provider = ai_get_provider(provider_name);
if (!provider) {
log_warning("Provider '%s' not found for setting '%s'", provider_name, setting);
return;
return FALSE;
}
/* settings is iterated by the request thread; mutate under settings_lock */
pthread_mutex_lock(&provider->settings_lock);
if (!provider->settings) {
provider->settings = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, g_free);
}
if (value) {
g_hash_table_insert(provider->settings, g_strdup(setting), g_strdup(value));
} else {
g_hash_table_remove(provider->settings, setting);
}
pthread_mutex_unlock(&provider->settings_lock);
if (value) {
prefs_ai_set_setting(provider_name, setting, value);
log_info("Setting '%s' for provider '%s' set to: %s", setting, provider_name, value);
} else {
g_hash_table_remove(provider->settings, setting);
prefs_ai_remove_setting(provider_name, setting);
log_info("Setting '%s' removed for provider '%s'", setting, provider_name);
}
return TRUE;
}
gchar*
@@ -766,8 +797,10 @@ ai_get_provider_setting(const gchar* provider_name, const gchar* setting)
if (!provider || !provider->settings)
return NULL;
gchar* value = g_hash_table_lookup(provider->settings, setting);
return g_strdup(value);
pthread_mutex_lock(&provider->settings_lock);
gchar* value = g_strdup(g_hash_table_lookup(provider->settings, setting));
pthread_mutex_unlock(&provider->settings_lock);
return value;
}
/* ========================================================================
@@ -1436,7 +1469,9 @@ ai_session_switch(AISession* session, const gchar* provider_name,
* the session lock while calling it.
*
* Custom provider settings are merged into the payload as additional JSON
* key-value pairs alongside "model" and "input".
* 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
@@ -1473,20 +1508,33 @@ _build_json_payload_from_list(AIProvider* provider, const gchar* model, GList* h
auto_gchar gchar* escaped_model = ai_json_escape(model);
/* Build custom settings JSON fragment */
/* Build custom settings JSON fragment; settings is mutated on the main
* thread (/ai set custom), so iterate under settings_lock */
GString* custom_json = g_string_new("");
if (provider && provider->settings) {
GHashTableIter iter;
gpointer key, value;
g_hash_table_iter_init(&iter, provider->settings);
while (g_hash_table_iter_next(&iter, &key, &value)) {
auto_gchar gchar* escaped_key = ai_json_escape((gchar*)key);
auto_gchar gchar* escaped_val = ai_json_escape((gchar*)value);
if (custom_json->len > 0) {
g_string_append_c(custom_json, ',');
if (provider) {
pthread_mutex_lock(&provider->settings_lock);
if (provider->settings) {
GHashTableIter iter;
gpointer key, value;
g_hash_table_iter_init(&iter, provider->settings);
while (g_hash_table_iter_next(&iter, &key, &value)) {
if (_is_reserved_payload_key((gchar*)key)) {
continue; /* fixed payload fields stay authoritative */
}
auto_gchar gchar* escaped_key = ai_json_escape((gchar*)key);
if (custom_json->len > 0) {
g_string_append_c(custom_json, ',');
}
if (_is_json_bare_token((gchar*)value)) {
g_string_append_printf(custom_json, "\"%s\": %s", escaped_key, (gchar*)value);
} else {
/* non-scalar values must be quoted, else the payload is invalid JSON */
auto_gchar gchar* escaped_val = ai_json_escape((gchar*)value);
g_string_append_printf(custom_json, "\"%s\": \"%s\"", escaped_key, escaped_val);
}
}
g_string_append_printf(custom_json, "\"%s\": %s", escaped_key, escaped_val);
}
pthread_mutex_unlock(&provider->settings_lock);
}
/* Assemble final payload: model, messages, custom settings, then fixed keys */

View File

@@ -17,14 +17,15 @@ typedef struct ai_message_t
*/
typedef struct ai_provider_t
{
gchar* name; /* Provider name (e.g., "openai", "perplexity") */
gchar* api_url; /* API endpoint URL */
gchar* project_id; /* Optional project ID (for some providers) */
gchar* default_model; /* Default model for this provider */
GHashTable* settings; /* Extensible per-provider settings (e.g., tools=enabled, search=disabled) */
GList* models; /* Cached models (gchar*) */
gboolean models_fresh; /* Whether models cache is current */
gint32 ref_count; /* Reference count (atomic) */
gchar* name; /* Provider name (e.g., "openai", "perplexity") */
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) */
GHashTable* settings; /* Extensible per-provider settings (e.g., temperature=0.7) */
GList* models; /* Cached models (gchar*) */
gboolean models_fresh; /* Whether models cache is current */
gint32 ref_count; /* Reference count (atomic) */
} AIProvider;
/**
@@ -142,11 +143,13 @@ const gchar* ai_get_provider_default_model(const gchar* provider_name);
/**
* Set a custom setting for a provider.
* Reserved payload keys (model, messages, stream) are rejected.
* @param provider_name The provider name
* @param setting The setting key (e.g., "tools", "search")
* @param value The setting value
* @param setting The setting key (e.g., "temperature")
* @param value The setting value, or NULL to remove
* @return TRUE on success, FALSE on unknown provider or reserved key
*/
void ai_set_provider_setting(const gchar* provider_name, const gchar* setting, const gchar* value);
gboolean ai_set_provider_setting(const gchar* provider_name, const gchar* setting, const gchar* value);
/**
* Get a custom setting for a provider.
@@ -184,9 +187,9 @@ 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 LLM response body. Tries Perplexity
* /v1/responses "text" first, falls back to OpenAI "content". Decodes
* the \" and \n escape sequences only.
* Extract assistant content from an OpenAI chat-completions response body
* ({"choices":[{"message":{"content":"..."}}]}). Decodes the \" and \n
* escape sequences only.
* @param response_json The JSON body from the provider
* @return Newly allocated content string, or NULL if not found (caller frees)
*/

View File

@@ -2833,7 +2833,7 @@ static const struct cmd_t command_defs[] = {
{ "set provider <name> <url>", "Add or update a provider with custom API endpoint" },
{ "set token <provider> <token>", "Set API token for a provider (e.g., openai, perplexity)" },
{ "set default-model <provider> <model>", "Set default model for a provider" },
{ "set custom <provider> <setting> <value>", "Set provider-level setting (e.g., tools, search)" },
{ "set custom <provider> <setting> <value>", "Set provider-level setting (e.g., temperature, max_tokens)" },
{ "remove provider <name>", "Remove a custom provider" },
{ "providers", "List configured providers with full details" },
{ "start <provider> [<model>]", "Start new AI chat (space-separated, uses defaults if omitted)" },
@@ -2846,7 +2846,7 @@ static const struct cmd_t command_defs[] = {
"/ai set token perplexity pplx-xxx",
"/ai set provider custom https://my-api.com/v1/chat/completions",
"/ai set default-model perplexity sonar",
"/ai set custom perplexity tools enabled",
"/ai set custom perplexity temperature 0.7",
"/ai remove provider custom",
"/ai start",
"/ai start perplexity",

View File

@@ -10787,8 +10787,11 @@ cmd_ai_set(ProfWin* window, const char* const command, gchar** args)
cons_bad_cmd_usage(command);
return TRUE;
}
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]);
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("");
return TRUE;
}

View File

@@ -614,6 +614,24 @@ test_ai_set_provider_setting(void** state)
assert_null(ai_get_provider_setting("p_set_settings", "nonexistent_setting"));
}
void
test_ai_set_provider_setting_reserved_key(void** state)
{
ai_add_provider("p_reserved_keys", "https://example.test/rk/");
/* 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", "stream", "true"));
assert_null(ai_get_provider_setting("p_reserved_keys", "model"));
/* 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");
assert_non_null(temp);
assert_string_equal("0.7", temp);
}
void
test_ai_get_provider_setting(void** state)
{
@@ -919,22 +937,22 @@ test_ai_parse_response_openai_content(void** state)
}
void
test_ai_parse_response_perplexity_text(void** state)
test_ai_parse_response_legacy_text_format_unsupported(void** state)
{
/* Legacy Perplexity /v1/agent format: "content" is an array, not a string.
* Dropped with the chat-completions alignment, so parsing yields NULL. */
const gchar* json = "{\"output\":[{\"content\":[{\"text\":\"Search result\",\"type\":\"output_text\"}]}]}";
auto_gchar gchar* out = ai_parse_response(json);
assert_non_null(out);
assert_string_equal("Search result", out);
assert_null(ai_parse_response(json));
}
void
test_ai_parse_response_text_preferred_over_content(void** state)
test_ai_parse_response_content_preferred_over_text(void** state)
{
/* When both formats are present, "text" wins (Perplexity path tried first). */
/* Chat-completions "content" is extracted; a stray "text" field is ignored. */
const gchar* json = "{\"text\":\"from text field\",\"content\":\"from content field\"}";
auto_gchar gchar* out = ai_parse_response(json);
assert_non_null(out);
assert_string_equal("from text field", out);
assert_string_equal("from content field", out);
}
void

View File

@@ -43,6 +43,7 @@ void test_ai_providers_find_case_insensitive(void** state);
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(void** state);
/* Model caching tests */
void test_ai_models_are_fresh_initial(void** state);
@@ -64,8 +65,8 @@ 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_perplexity_text(void** state);
void test_ai_parse_response_text_preferred_over_content(void** state);
void test_ai_parse_response_legacy_text_format_unsupported(void** state);
void test_ai_parse_response_content_preferred_over_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);

View File

@@ -752,6 +752,7 @@ main(int argc, char* argv[])
cmocka_unit_test_setup_teardown(test_ai_set_provider_default_model, ai_client_setup, ai_client_teardown),
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, 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),
@@ -774,8 +775,8 @@ 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_perplexity_text),
cmocka_unit_test(test_ai_parse_response_text_preferred_over_content),
cmocka_unit_test(test_ai_parse_response_legacy_text_format_unsupported),
cmocka_unit_test(test_ai_parse_response_content_preferred_over_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),