diff --git a/src/ai/ai_client.c b/src/ai/ai_client.c index 0c477814..4ca329d9 100644 --- a/src/ai/ai_client.c +++ b/src/ai/ai_client.c @@ -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 */ diff --git a/src/ai/ai_client.h b/src/ai/ai_client.h index 98d0903a..7898dbfc 100644 --- a/src/ai/ai_client.h +++ b/src/ai/ai_client.h @@ -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) */ diff --git a/src/command/cmd_defs.c b/src/command/cmd_defs.c index ccd9f1da..795d0547 100644 --- a/src/command/cmd_defs.c +++ b/src/command/cmd_defs.c @@ -2833,7 +2833,7 @@ static const struct cmd_t command_defs[] = { { "set provider ", "Add or update a provider with custom API endpoint" }, { "set token ", "Set API token for a provider (e.g., openai, perplexity)" }, { "set default-model ", "Set default model for a provider" }, - { "set custom ", "Set provider-level setting (e.g., tools, search)" }, + { "set custom ", "Set provider-level setting (e.g., temperature, max_tokens)" }, { "remove provider ", "Remove a custom provider" }, { "providers", "List configured providers with full details" }, { "start []", "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", diff --git a/src/command/cmd_funcs.c b/src/command/cmd_funcs.c index 7fbc5f05..d9293e66 100644 --- a/src/command/cmd_funcs.c +++ b/src/command/cmd_funcs.c @@ -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; } diff --git a/tests/unittests/test_ai_client.c b/tests/unittests/test_ai_client.c index 4a40b0d4..516fcb7f 100644 --- a/tests/unittests/test_ai_client.c +++ b/tests/unittests/test_ai_client.c @@ -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 diff --git a/tests/unittests/test_ai_client.h b/tests/unittests/test_ai_client.h index e99c355d..6bf2062e 100644 --- a/tests/unittests/test_ai_client.h +++ b/tests/unittests/test_ai_client.h @@ -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); diff --git a/tests/unittests/unittests.c b/tests/unittests/unittests.c index ca1a7d1e..cf01788e 100644 --- a/tests/unittests/unittests.c +++ b/tests/unittests/unittests.c @@ -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),