The defaults_initialized sentinel only makes sense when prefs is loaded.
Three unit tests use ai_client_setup, which calls ai_client_init without
loading prefs — prefs_ai_get_providers was bailing on the seeding branch
because of the prefs NULL check, leaving ai_client with zero providers
and breaking tests that expect at least one default.
Decouple the two concerns:
- Always seed openai/perplexity into the in-memory result list when the
configured set is empty and we haven't already seeded.
- Only persist the seeded providers and write the defaults_initialized
flag when prefs is actually available.
In production this is identical to the previous behaviour; in tests
without prefs the old "always-return-defaults" semantics is preserved
without a persistence side effect.
Also reflow ai_client.c JSON escape switch and HTTP-error ternary to
satisfy clang-format 18 (CI checks).
Pull in the defaults-agnostic unit and functional test coverage so this
branch carries both the source fixes and the tests that exercise them.
# Conflicts:
# src/ai/ai_client.c
Per-provider state used to live as flat keys in [ai] with naming
conventions to avoid collisions: <name>_url, <name>_models,
<name>_default_model, and the bare <name> for the API token. Custom
settings (/ai set custom) never made it to disk at all — they only
lived in the runtime AIProvider.settings hash table and were lost on
restart.
Move every per-provider field into a [ai/<name>] subsection of the
keyfile with stable key names:
[ai/openai]
url=https://api.openai.com/
key=sk-...
default_model=gpt-4
models=gpt-4;gpt-3.5-turbo
setting.tools=enabled
[ai] now keeps only the global "defaults_initialized" sentinel.
Migration runs once in _prefs_load: collect provider names from the
old <name>_url keys, then move url/models/default_model/token of each
provider into its subsection and drop the flat keys. Idempotent on
subsequent loads.
prefs_ai_* API surface kept compatible; storage backend changes only.
Adds:
prefs_ai_remove_section() — wipe whole subsection in one call
prefs_ai_{set,get,remove,list}_setting() — persist custom settings
ai_client wiring:
- ai_set_provider_setting now writes to prefs as well as the runtime
hash table; ai_client_init loads them back on startup.
- ai_remove_provider replaces four prefs_ai_remove_* calls with a
single prefs_ai_remove_section.
Both the chat response parser and the error envelope parser were doing
their own ad-hoc strstr scans and inline unescape loops, with different
sets of supported escape sequences (chat: \" \n; error: \" \n \\ \t).
Models endpoint errors didn't try to extract error.message at all and
surfaced raw response bodies to the user — inconsistent with the chat
path which already did parse them.
Consolidate around two helpers:
_json_unescape_substring: decode \" \\ \/ \n \t \r \b \f into a fresh
buffer; pass \uXXXX through verbatim (out of scope here).
_extract_json_string: locate "field":"..." via _find_json_field
(already handles JSON whitespace around the colon) and return the
unescaped value, treating any \X inside the string body as opaque
so embedded \" is never misread as the closing quote.
_parse_ai_response and _parse_error_response now each collapse to a
small wrapper around these helpers. Wire _parse_error_response into the
shared _curl_exec_and_handle 4xx branch so models requests also surface
the provider's error.message instead of the raw body.
- Persist per-provider default model in [ai] prefs (set/get/remove
helpers, load on init, clear with provider removal). Previously
/ai set default-model only updated runtime state.
- Stop re-seeding openai/perplexity into [ai] every time the configured
provider list is empty. The defaults are now seeded once, guarded by
a "defaults_initialized" sentinel, so removing every provider does
not silently restore them.
- /ai providers (no arg) now lists the configured providers with key
status, instead of the hardcoded openai/perplexity blurb.
- /ai models <provider> (fresh fetch) prints the model names it cached,
not just a count. Previously the user had to follow up with --cached
to actually see them.
- /ai remove provider now also clears the persisted default model,
token, and cached model list, leaving no orphan keys in [ai].
- Drop dead ai_models_find declaration from ai_client.h (no
implementation in this tree).
Self-setup providers in tests instead of relying on hardcoded openai/perplexity
defaults. Multi-provider tests now use distinct URLs per provider; functional
tests check for http/https URL presence rather than specific provider names.
Drop ai_models_find tests (function removed upstream in feat/ai). Replace with
reset-hook + persistence coverage: providers_reset_ac restart cycle, provider
add/remove round-trip across init, model cache round-trip across init.
Keep three autocomplete prefix-change tests red in-suite (documented as latent
API-hygiene gap, unreachable from UI today). NULL-search test body retained
but registration commented out — currently SIGSEGVs the cmocka runner.
Add stub_xmpp connection_create_stanza_id to satisfy new cmd_funcs.c call site.
Adds 10 tests for ai_models_find and additional edge cases on
ai_providers_find. 5 pass as sanity; 5 fail intentionally, documenting
two distinct bugs in the autocomplete integration:
BUG A — ai_models_find cycling is broken
src/ai/ai_client.c ai_models_find() allocates a fresh Autocomplete on
every call and frees it before returning. The Autocomplete's last_found
cursor is therefore never persisted, so repeated calls with the same
prefix always return the first match. Tab+Tab+Tab cannot reach models
2, 3, ...
Caught by:
- test_ai_models_find_cycles_through_matches
- test_ai_models_find_null_search_cycles_all
BUG B — providers_ac is never reset between user input cycles
src/ai/ai_client.c keeps the static providers_ac AC, but
src/command/cmd_ac.c's all_acs[] (which cmd_ac_reset() iterates) does
not include &providers_ac. So when the input loop fires the global
reset, providers_ac retains its last_found cursor from the previous
prefix.
Result: tab-completing a new prefix continues from the old cursor
position instead of restarting at the head of the new prefix's matches.
A provider added mid-session may also be missed depending on cursor
position.
Caught by:
- test_ai_providers_find_state_resets_on_prefix_change
- test_ai_providers_find_empty_then_prefix
- test_ai_providers_find_after_add_includes_new
Sanity (5 passing):
- test_ai_models_find_no_models_returns_null
- test_ai_models_find_returns_match_for_prefix
- test_ai_models_find_no_match_returns_null
- test_ai_models_find_previous_direction
- test_ai_providers_find_after_remove_skips_removed
The failing tests are checked in as-is — they go green when the bugs
are fixed. See PR description for suggested fixes.
Unit tests (tests/unittests/test_ai_client.c): +50 cases on top of the
existing 50 — total 100. Covers:
- chat response parser (ai_parse_response): OpenAI content + Perplexity
text formats, escape decoding, empty/null/missing inputs, format-string
safety, multiline content
- error envelope parser (ai_parse_error_message): standard envelope,
nested escapes, missing fields, null/empty
- extended JSON escape: \b, \f, \r, all-specials, UTF-8 pass-through
- provider autocomplete cycling with >=2 matches and wrap-around
- session edge cases: NULL args, 100-message order preservation,
set_model(NULL), ref/unref(NULL)
- provider edge cases: get/remove with NULL, double-remove, survival
via session ref after ai_remove_provider
- settings: multi-key independence, missing key, cross-provider
isolation
- model parsing edges: data not array, empty data, "id" outside data,
multiple models
- prefs round-trip: set token -> shutdown -> init -> token reloaded
from disk (uses load_preferences fixture)
Functional tests:
- Console only (test_ai.c): 15 cases for the /ai command surface that
don't need HTTP. Covers /ai help, providers list, set provider/token,
start with/without key/unknown provider, clear, remove, default
provider/model, switch without window, bad subcommand.
Infrastructure:
- TEST_GROUPS bumped to 5 in proftest.c; AI tests live in their own
Group 5 because mixing them with stabber-driven tests in Group 4
poisoned stbbr_stop() teardown.
- PROF_FUNC_TEST_AI macro in functionaltests.c registers AI tests
with ai_init_test() which wraps init_prof_test with a prof_connect()
so stabber sees a graceful disconnect at teardown.
Source-level changes to enable testing:
- src/ai/ai_client.{c,h}: dropped 'static' from _parse_ai_response
(renamed ai_parse_response) and _parse_error_response (renamed
ai_parse_error_message); declared under a "Parsing helpers (exposed
for testing)" section. Same approach as ai_parse_models_from_json.
Results in Docker (cproof-debian image):
- 595/595 unit tests pass
- 15/15 AI functional tests pass (Group 5)
The user message was being added to session->history before calling
_build_json_payload_from_list(), which independently appends the prompt
to the JSON payload. This caused the same message to appear twice in
the API request.
Fix: Build JSON payload first, then add user message to history under
the same lock to maintain atomicity.
- Replace _ai_model_autocomplete() with new _ai_provider_and_model_autocomplete()
that handles both provider and model autocomplete in one function
- Add /ai set default-model model autocomplete support
- Remove trailing space requirement from cmd_prefix parameter (use \"/ai start\"
instead of \"/ai start \")
- Fix const correctness in autocomplete_param_with_func() signature
- Remove unused space_at_end variable
Remove window type check from cmd_ai_models() so fetching models works
from console. Response is shown via cons_show() when called from console,
or in the AI window when called from there.
Fix /ai set provider <name> <url> not persisting across restarts, and
default providers (openai, perplexity) disappearing when user adds a
custom provider.
- Add prefs_ai_set_provider(), prefs_ai_remove_provider(),
prefs_ai_get_provider_url(), prefs_ai_list_providers(),
prefs_free_ai_providers() for provider persistence.
- Implement prefs_ai_get_providers() in preferences.c: if no providers
are configured, write default providers to config and return them.
This ensures defaults are persisted so users can modify them directly.
- Move default provider URL definitions from ai_client.c to
preferences.c. ai_client.c no longer knows about specific provider
names.
- Simplify ai_client_init() to call prefs_ai_get_providers() and
initialize with whatever it receives.
- ai_add_provider() persists to config; ai_remove_provider() removes
from config.
Behavior: On first run, default providers are written to config. User
can then modify, remove, or add providers freely and they persist.
Fix /ai set provider <name> <url> not persisting across restarts.
- Add prefs_ai_set_provider(), prefs_ai_remove_provider(),
prefs_ai_get_provider_url(), prefs_ai_list_providers(),
prefs_free_ai_providers(), and prefs_ai_get_default_provider_url()
to preferences.h/c for provider persistence.
- Move default provider URL definitions (openai, perplexity) from
ai_client.c to preferences.c, following the established pattern
where defaults are returned as fallbacks when no config value exists.
- Modify ai_add_provider() to persist new/updated providers to config.
- Modify ai_remove_provider() to remove provider from config.
- Add internal _ai_add_provider_nopersist() helper for adding default
providers and loading from config without double-persisting.
- Update ai_client_init() to load custom providers from config on
startup, allowing users to override defaults or add new ones.
Users now have full control: default providers remain as fallbacks,
user-added providers persist to config, and users can override or
remove defaults freely.
Replace the custom ai_models_find function with a dedicated
ai_models_ac autocomplete list. Update the /ai switch trigger to
include a trailing space for consistent parsing.
- Remove `/ai set default-provider` command and related autocomplete logic
- Remove global `wins_get_ai()` window accessor function
- Enforce strict requirement that `/ai model`, `/ai switch`, `/ai models`, and `/ai clear` commands must be executed from within an active AI chat window
- Update `/ai start` to require `<provider>` argument
BREAKING CHANGE: `/ai model`, `/ai switch`, `/ai models`, and `/ai clear` commands now require an active AI window context. The `/ai set default-provider` command has been removed.
When typing /ai start <prefix><tab>, the autocomplete was not properly
handling search string changes. For example, typing "/ai start o<tab>"
would return "openai", but then typing "/ai start p<tab>" would still
return "openai" because the autocomplete state was not reset.
This commit:
- Adds ai_providers_reset_ac() function to reset provider autocomplete
state, following the same pattern as accounts_reset_all_search()
- Integrates ai_providers_reset_ac() into cmd_ac_reset() for proper
state cleanup when user presses Ctrl+C or switches contexts
cmd_ai_switch() and cmd_ai_clear() mutated session fields (provider_name,
provider, model, api_key, history) on the main thread without coordination,
while _ai_request_thread() read the same fields concurrently in the worker
thread. This caused use-after-free when:
- g_free(session->model) was called between worker's read and g_strdup()
- ai_provider_unref(session->provider) freed the provider struct while
worker accessed session->provider->api_url
- ai_session_clear_history() freed the history list while worker walked it
Fix:
1. Add pthread_mutex_t lock to AISession struct to protect all session
fields from concurrent access.
2. Introduce ai_session_switch() public API that atomically switches
provider, model, and API key under the session lock. This encapsulates
all session mutations within ai_client.c so cmd_funcs.c never touches
session fields directly.
3. Have _ai_request_thread() snapshot all session state under the session
lock before making requests, using local copies for the duration of
the curl request.
4. Add ai_provider_ref()/ai_provider_unref() around _ai_generic_request_thread()
to prevent provider UAF during model-fetch requests.
5. Protect ai_session_add_message(), ai_session_clear_history(), and
ai_session_set_model() with the session lock.
No pthread.h needed in cmd_funcs.c — all locking is encapsulated within
ai_client.c via the public API. No deadlock risk from nested locks.
Files changed:
- src/ai/ai_client.h: Add lock field, ai_session_switch() declaration
- src/ai/ai_client.c: Mutex init/destroy, session mutation protection,
worker thread snapshot, provider ref management
- src/command/cmd_funcs.c: Use ai_session_switch() API, remove pthread.h
Introduce a pthread mutex to protect all AISession fields from
concurrent access between the main thread and background request
worker.
- Initialize and destroy mutex during session lifecycle
- Lock session state when modifying or reading history, model, and
provider fields
- Snapshot session state under lock in the request worker thread to
prevent UAF races during API calls
- Refactor _build_json_payload to accept direct parameters for safe
usage under lock
- Update command handlers to safely switch provider/model under lock
The org_id field was removed from AIProvider as it was no longer
populated through the command surface. The /ai set org command was
previously removed, leaving org_id as dead code.
This change removes org_id from:
- AIProvider struct definition
- ai_provider_new() and ai_provider_unref()
- ai_add_provider() API signature
- _build_curl_headers() (OpenAI-Organization header)
- cmd_ai_set() and cmd_ai_providers()
- All unit tests
src/command/cmd_funcs:11118 used a plain non-atomic ref_count++ on
ai_provider, which is a data race against concurrent
ai_provider_unref() calls using g_atomic_int_dec_and_test.
- Expose ai_provider_ref() in ai_client.h (was static)
- Replace non-atomic ref_count++ with ai_provider_ref() in cmd_ai_switch"
_aiwin_validate() no longer acquires the mutex. Callers now hold the
lock across both the existence check and the display dispatch,
eliminating the TOCTOU window where the window could be freed.
When the user scrolls up to view history, the paged flag is set to 1.
This suppresses new messages in _win_printf(). Reset paged and
unread_msg flags before printing to ensure visibility.
Add early return for empty responses and fallback to console output
when the AI window is closed or invalid. Implement JSON error
parsing to extract readable messages from API failures, improving
debugging and user feedback.
Rewrote internal JSON parsing to correctly handle whitespace, escaped
quotes, and strict field context extraction. This prevents incorrect
field names or provider names from being added to the model list.
Added `ai_parse_models_from_json` public API to facilitate testing.
Changed `/ai models` default behavior to always fetch fresh models.
Replaced `--refresh` flag with `--cached` to display local cache.
Added comprehensive unit tests covering OpenAI and Perplexity formats,
array format, empty/null JSON, escaped quotes, and whitespace handling.
The existence check accesses shared state without synchronization,
which can cause race conditions when called from background threads.
Explicitly locking the mutex ensures safe access during validation.
- Introduce model caching with persistence to preferences
- Add provider default model and custom settings management
- Implement `/ai switch`, `/ai models`, and improve `/ai start`
- Add model name autocomplete for chat commands
- Update command definitions and help text
- Add unit tests for new functionality
Return CURL_WRITEFUNC_ERROR instead of realsize when the response
exceeds 10MB. This causes curl to abort the transfer immediately
instead of continuing to stream data.
Replace plain ref_count++/-- with g_atomic_int_inc and
g_atomic_int_dec_and_test for both AIProvider and AISession refcounts.
Prevents data races when ref/unref is called from worker threads.
Add store:false to AI API requests to prevent OpenAI, Perplexity, and
other providers from storing conversation data or using it for training.
Both the OpenAI Responses API and Perplexity API support the store
parameter to control whether requests are persisted. Setting it to false
ensures conversations remain private and are not retained by the provider.
Privacy:
- Requests are not stored by AI providers
- Conversations are not used for model training
- Aligns with CProof's privacy-first design
Prevent use-after-free in the AI request worker thread when the user
closes the AI window during the 60-second HTTP request timeout. Without
this fix, the worker may dereference a dangling pointer and crash or
exhibit undefined behavior.
The _ai_request_thread function in ai_client.c previously cast user_data
to ProfAiWin* and used it directly without verifying the window was still
alive. Added wins_ai_exists() to iterate through all AI windows and check
if the pointer is still valid before use.
Safety:
- wins_ai_exists() iterates all AI windows (handles multiple AI windows)
- Worker skips UI update if window was freed, just cleans up
- Logs warning when dangling pointer is detected
The ai_response_cb and ai_error_cb parameters were always passed the
same functions (aiwin_display_response and aiwin_display_error),
making the callback indirection redundant and complicating the call
chain.
Remove ai_callback_data_t, _ai_callback_invoke(), and
_ai_invoke_callback(). Simplify _ai_request_thread and ai_send_prompt
to directly call aiwin_display_error() and aiwin_display_response()
when user_data is a valid ProfAiWin*.
Update ai_providers_find() to handle NULL search_str safely by treating it
as an empty string, preventing a segfault in strdup() within autocomplete_complete().
Rename test_ai_providers_find_case_sensitive to test_ai_providers_find_case_insensitive
to reflect the new case-insensitive matching contract. Update all affected tests to
expect cycling behavior (NULL/empty returns first provider) and use auto_gchar for
automatic memory management.
Use `gchar` instead of `char` in ai_providers_find for consistency
Take the global lock mutex before invoking all callbacks (success
and error) to ensure thread-safety. Previously some paths took the
lock and others did not, creating asymmetric behavior that could
lead to data races when callbacks access UI state.
Replace the manual GHashTable iteration in ai_providers_find() with the
existing stateful autocomplete system. Provider names are now kept in
sync via autocomplete_add/remove during ai_add_provider/ai_remove_provider,
and ai_providers_find delegates to autocomplete_complete() for deterministic
tab-completion cycling through sorted results.
This eliminates manual GList allocation, iteration, and cleanup, while
providing consistent forward/backward cycling behavior shared by all
autocomplete callers.
Replace the deeply nested conditional tree in _ai_autocomplete() with a
flat sequential chain of autocomplete_param_with_*() calls. Each call
tries a specific command prefix (e.g., "/ai set provider", "/ai start")
and returns immediately on match — a common, reliable pattern for
command-line tab completion.
The previous design nested provider-name autocomplete inside
g_strv_length() and g_strcmp0() checks, meaning /ai<tab> and /ai <tab>
would never reach the subcommand matcher. The new flat structure ensures
every prefix is tried in order, with the most specific prefixes first
and the generic /ai subcommand fallback last.
This pattern — sequential prefix matching with early return — is the
standard approach for autocomplete dispatch: each handler owns its
prefix, no shared state or argument parsing is needed, and adding a
new subcommand is a single append to the chain.
The generic `/ai <subcommand>` tab-completion was nested inside conditional
blocks that prevented it from firing in common cases — for example,
`/ai<tab>` or `/ai <tab>` would never match subcommands because the
`autocomplete_param_with_ac()` call was guarded by argument count checks
and nested inside the explicit handler branches.
Move the fallback subcommand autocomplete to execute unconditionally after
all explicit handler branches, ensuring `/ai <tab>` always lists available
subcommands regardless of argument count or which subcommand was typed.
Also move the `/ai set` subcommand fallback outside the inner else-block
so it runs whenever args[0] is "set", not only when num_args >= 2.
Use g_strjoinv to join all arguments from args[1] onwards into a
single prompt string. Previously only args[1] was used, truncating
multi-word prompts like '/ai correct hello world' to just 'hello'
Use auto_gchar temporary for g_strndup result in cmd_ai_start to
prevent memory leak. Fix const-cast warning by using a proper
non-const temporary variable.
Remove ai_provider_ref() from ai_list_providers() so the returned
providers do not need to be unref'd by the caller. This matches the
header docstring which states "caller must not free the list or
providers". Also update all callers (cmd_ai_providers, tests) to
remove redundant ai_provider_unref() calls.
ai_get_provider_key() already returns a freshly g_strdup'd copy of
the API key. The outer g_strdup in ai_session_create() created an
unnecessary second copy, leaking the original pointer.
Remove the log_debug line in ai_client.c that leaked the first 10
characters of the API key into logs. This is also contained a memory leak (g_strndup
return value was never freed).
API keys set via /ai set token were only stored in memory and lost
on client restart. prefs_ai_set_token() and prefs_ai_remove_token()
mutated the in-memory prefs but never called _save_prefs(), unlike
other preference setters in the same file.
Both functions now call _save_prefs() after mutating the keyfile,
ensuring API keys persist across client restarts as advertised.
Add an AI client module that integrates with OpenAI-compatible API
providers (OpenAI, Perplexity, and custom providers) to provide
AI-assisted responses within the profanity client.
The implementation includes:
- src/ai/ai_client.c/h: Core AI client with provider management,
session handling, and async HTTP request handling via libcurl.
Supports per-provider API keys stored in preferences, reference-
counted sessions, and conversation history tracking.
- src/ui/window.c/window_list.c: New AI window type (ProfAiWin) for
displaying AI conversations, with response streaming and error
display capabilities.
- Command integration: New `/ai` command (cmd_defs.c, cmd_funcs.c)
for creating sessions, sending prompts, and managing providers.
Provider autocomplete support in cmd_ac.c.
- Preferences integration: API keys for providers are persisted in
the preferences system (config/preferences.c).
- Unit tests: 472 lines of comprehensive tests covering provider
management, session lifecycle, JSON escaping, and autocomplete
(tests/unittests/test_ai_client.c).
Architecture decisions:
- Asynchronous design: HTTP requests run on a separate thread to
avoid blocking the main UI loop. Callbacks are invoked on the main
thread via direct function call (profanity uses ncurses, not GLib
main loop).
- Reference counting: Both AIProvider and AISession use ref counting
for safe shared ownership.
- Response size limit: 10MB cap on HTTP responses to prevent OOM.
2026-04-29 19:23:33 +00:00
5 changed files with 39 additions and 65 deletions
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
