feat(ai): overhaul the AI subsystem - #79
Conversation
Anthropic had never once produced a commit message: its tool loop sent
`messages: []` on the first iteration, which the Messages API rejects with a
400, and `use_tools` defaults to true so that was the default path. The
shipped default model was a retired id that appeared in no picker, so a fresh
install rendered the Model row with nothing selected at all. The model pill
row had no `flex_wrap()`, so the default window could not render the default
provider's own model list. `get_file_content` correctly rejected `../.env` and
then happily read `.env`. All four are fixed here, along with the structural
problems underneath them.
Correctness
- Seed an opening user turn for every provider, guarded by a test asserting
no provider sends an empty conversation on iteration 1.
- Replace string provider dispatch with an `AiProvider` enum owning endpoint,
auth shape and default model; unknown ids in settings.json are reported
through `load_warnings` instead of failing at click time.
- Make every Gemini response field optional and iterate all parts, so a
safety block, a `MAX_TOKENS` stop, a leading thought part and parallel
function calls stop collapsing into one opaque parse error.
- Bail on a `tool_calls` finish with an empty array rather than burning
iterations to reach a generic message.
- Cap response bodies at 8 MB, move the Gemini key from the query string to
`x-goog-api-key`, and use `from_utf8_lossy` in the diff walker so non-UTF-8
lines stop vanishing from what the model reasons about.
Threading and lifecycle
- Run the whole provider dispatch, tool execution and JSON parsing on the
background executor, reporting tool progress over an mpsc channel that a
small foreground task drains. `execute_tool` spawns `git` and walks
directories synchronously; none of that belongs on the render thread.
- Give every request a 60s deadline and hold its `Task`, so a stalled provider
no longer leaves the spinner running until the app is restarted, and a
cancel button can drop it.
- Carry a `GenerationId { sequence, repo_path }` on every event and route by
repo path rather than `active_tab`, so a message generated for one tab
cannot land in another's commit box.
- Stamp the cooldown on completion rather than dispatch, and report it as
info without clearing an in-flight generation's spinner.
- Skip unchanged keychain writes, debounce the rest by 400ms, and resolve all
providers' keys in one pass.
- Retry 429/5xx honouring `Retry-After`, set an Anthropic cache breakpoint
after the system prompt, lower the diff cap to 40 KB, and give tool output a
per-generation budget.
Security
- Deny `.git/`, a credentials denylist, git-ignored paths and non-UTF-8
content before `get_file_content` reads anything, and re-check the
canonicalised path so an in-repo symlink cannot reach `.git/config`.
Per-provider credentials
- One keychain slot per provider (`ai/provider/{id}`), migrated from the
single `ai/default` slot, with per-provider model pins so switching provider
no longer destroys the previous provider's key or its model choice.
Features
- Live model catalogue with stale-while-revalidate caching, a bundled
fallback, and a generation guard; the picker labels which source it is
showing so "three weeks old" is distinguishable from "shipped with the app".
- OpenRouter, via the existing OpenAI-compatible path widened with an endpoint
struct; the tool-support 404 gets an actionable message.
- `base_url_override` for the OpenAI-compatible family, validated, with an
inline warning naming the host the key will be sent to.
UI
- Provider accordion replacing six flat cards, each row owning its key field,
connection status and model pin, with a connection test that can tell a
working key from a typo.
- `flex_wrap()` on `pill_group` and `overflow_hidden` on `setting_card`.
- The AI button states its own reason when it cannot be used, stays reachable
when the fix is one click away, and shows the shortcut that was registered
all along.
- Tool progress routed to the panel chip, cancel, regenerate with a style
override, and undo of an AI overwrite.
- Errors stop auto-dismissing at 3s and can carry an action; per-level toast
durations replace the single hardcoded one.
- Delete the Save button: text inputs now flush on Enter and on blur, so the
same value no longer persists or vanishes depending on how it arrived.
- `tab_index` across the settings page, which previously had none.
Removals
- `last_result`/`last_error` had no callers; `uuid` was one call site whose id
was never transmitted; `http_client` was never referenced by path.
- `fuzzy_score` is now one shared implementation rather than three.
Not included: streaming tokens into the summary field. It needs per-provider
SSE parsing including tool-call deltas across three wire formats, with no way
to exercise it offline; the progress trace covers the feedback gap meanwhile.
The recommended model ids come from the plan's research and have not been
verified against a live API. A wrong id is now visible and correctable in the
picker rather than silently unselectable, but each still wants one real
request to confirm.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff71bae62d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Request::builder(), | ||
| ), | ||
| AiProvider::OpenAi => ( | ||
| "https://api.openai.com/v1/models".to_string(), |
There was a problem hiding this comment.
When an OpenAI-compatible provider is configured with a custom gateway, automatic catalogue refreshes and “Test connection” still send the provider's stored key to this hardcoded official endpoint. Opening the AI settings can therefore disclose a gateway credential to OpenAI and report a failed connection even though generation correctly targets the override; derive the /models endpoint from base_url_override as well.
Useful? React with 👍 / 👎.
| .find(|tab| tab.effective_repo_path(cx) == id.repo_path) | ||
| .map(|tab| tab.commit_panel.clone()) |
There was a problem hiding this comment.
If the user enters, leaves, or switches an inspected worktree while generation is running, effective_repo_path no longer equals the path captured in GenerationId. Subsequent progress/completion events then find no panel, so the generated message is discarded and the original commit panel can remain stuck in its generating state; capture the originating panel/tab identity rather than looking it up through mutable inspection state.
Useful? React with 👍 / 👎.
| if let Some(panel) = commit_panel_for(this, id, cx) { | ||
| panel.update(cx, |cp, cx| cp.fail_ai_generation(cx)); |
There was a problem hiding this comment.
When the user explicitly cancels a generation, this routes the cancellation through fail_ai_generation, leaving the panel displaying the red “AI failed — retry” control even though no failure occurred. Cancellation should clear the generating state without marking the operation as failed.
Useful? React with 👍 / 👎.
| let bare = host.split(':').next().unwrap_or(host); | ||
| matches!(bare, "localhost" | "127.0.0.1" | "[::1]" | "::1") |
There was a problem hiding this comment.
For the valid local endpoint http://[::1]:11434/v1, splitting at the first colon produces "[", so is_loopback_host rejects it as insecure even though ::1 is explicitly listed as allowed. Parse the bracketed authority before removing the port so IPv6-only Ollama or other local gateways can be configured.
Useful? React with 👍 / 👎.
Honour `base_url_override` when fetching a model catalogue. The `/models` request went to the official host unconditionally, so opening the AI settings sent a gateway-only key to OpenAI and then reported the provider as unreachable while generation was correctly reaching the gateway. The catalogue cache is keyed by the resolved endpoint too, so removing an override no longer leaves the gateway's models on screen, and changing one drops the stale catalogue and connection result for the family. Route AI generation events by the panel captured at dispatch rather than by looking a tab up through `effective_repo_path`. That path moves when the user enters or leaves an inspected worktree, so a generation that outlived that click matched no tab: the message was discarded and the originating panel spun forever. Return the panel to idle on cancellation instead of to `Failed`. Routing a cancel through `fail_ai_generation` left a red "AI failed — retry" control on screen after the user had done exactly what they intended. Parse bracketed IPv6 authorities before stripping the port, so `http://[::1]:11434/v1` — an IPv6-only Ollama — validates as loopback instead of being rejected as insecure. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@codex review |
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39797c03d2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ai.enabled = self.ai_enabled; | ||
| ai.inject_project_context = self.ai_inject_project_context; | ||
| ai.use_tools = self.ai_use_tools; | ||
| ai.base_url_override = self.ai_base_url_override.trim().to_string(); |
There was a problem hiding this comment.
After a user types an invalid override such as http://gateway.example.com/v1, the Changed handler stores it in self.ai_base_url_override; although blur displays a validation error, any subsequent checkbox, provider switch, or window-close save reaches this unconditional assignment and persists the rejected value. Generation does not re-run validate_base_url, so it can later send the provider's API key over the insecure endpoint that the UI claimed to reject. Validate here before mutating settings, or keep draft and committed values separate.
Useful? React with 👍 / 👎.
| if value == self.ai_base_url_override { | ||
| return; |
There was a problem hiding this comment.
For every normal edit, the Changed subscription has already copied the editor text into self.ai_base_url_override, so this equality is always true when Submit or Blurred calls commit_base_url_override. The function therefore skips both save_settings and catalogue/connection invalidation: pressing Enter can leave generation using the old endpoint while the connection test uses the new local value, and a later unrelated save persists it without performing the intended invalidation.
Useful? React with 👍 / 👎.
| if !force && self.ai_catalog_loading { | ||
| return; |
There was a problem hiding this comment.
If provider A is refreshing and the user expands provider B before it finishes, B's load_ai_catalog installs its cached/bundled rows but this process-wide loading flag suppresses B's network refresh. A's completion only clears the flag and applies A's result; nothing retries B, so the newly expanded provider remains on its fallback catalogue until the user manually presses Refresh. Track the in-flight provider or supersede the previous request instead of treating all providers as one load.
Useful? React with 👍 / 👎.
| .tab_index(tab_base + 4) | ||
| .on_click(cx.listener(move |this, _: &ClickEvent, _, cx| { |
There was a problem hiding this comment.
When navigating Settings with a keyboard, this field can receive focus through tab_index, but it only installs an on_click handler; Enter and Space therefore do nothing and the model picker cannot be opened without a pointer. Add a key handler or use a button-like component that maps keyboard activation to the same action.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not applying this one — the handler would break the field rather than fix it.
gpui already maps Enter and Space to a click on any focused element that has click listeners. tab_index() sets focusable = true (gpui elements/div.rs, fn tab_index), which installs a tracked focus handle, and the paint path guarded by if is_focused registers a KeyDownEvent listener that arms on enter/space plus a KeyUpEvent listener that synthesises ClickEvent::Keyboard and runs the same click_listeners. The model field's on_click — which toggles ai_model_picker_open — therefore already fires from the keyboard.
Adding an on_key_down that toggles as well would fire on key-down and again on the synthetic click at key-up: the picker would open and immediately close.
The other three findings are fixed in 63246b9.
…per provider The base URL editor mirrored every keystroke into the field `save_settings` persists, so a URL the UI had just rejected still reached `settings.json` on the next unrelated save — and from there the provider's API key reached the insecure endpoint. The draft now lives in the editor until Enter or blur, and only a validated value is copied out, which also makes the "unchanged" check in `commit_base_url_override` compare against the committed value rather than against itself, so Enter actually saves and invalidates. `normalize_base_url` now rejects anything `validate_base_url` would. It is the one choke point every consumer goes through, so a plain-http gateway cannot become a live endpoint by a route that skips the field — a hand-edited settings.json included. Catalogue loading is tracked per provider. One shared flag and one shared task slot meant expanding a second provider cancelled the first's fetch and then suppressed its own, leaving the newly opened row on its bundled list until the user pressed Refresh by hand; the refresh error is per provider too, so one row's failure no longer annotates another's picker. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 63246b9c68
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| extra_headers: &[(&str, &str)], | ||
| body: &Value, | ||
| ) -> Result<Value> { | ||
| let key = api_key(req)?; |
There was a problem hiding this comment.
When an OpenAI-compatible override points to a keyless local service such as Ollama, this unconditional key lookup rejects generation before any request is sent. The settings and commit-panel guards likewise require a stored key, so the newly supported Ollama base URL cannot actually be used unless the user invents and stores a dummy credential; custom endpoints should be allowed to omit the authorization header when no key is configured.
Useful? React with 👍 / 👎.
| // official host instead sent a gateway-only key to the provider and | ||
| // then called a working configuration broken. | ||
| let base_url = self.ai_base_url_override.clone(); | ||
| self.ai_test_task = Some(cx.spawn(async move |this, cx: &mut gpui::AsyncApp| { |
There was a problem hiding this comment.
If provider A is being tested and the user starts a test for provider B, assigning this single task slot drops and cancels A's task after A was marked Testing. Only B's completion then updates state, leaving A permanently displayed as Testing and its Test button disabled when its row is reopened; store tasks per provider or explicitly restore the superseded provider's state.
Useful? React with 👍 / 👎.
| .on_click(cx.listener( | ||
| move |this, _: &ClickEvent, _, cx| { | ||
| cx.stop_propagation(); | ||
| this.select_ai_model(suggestion.clone(), cx); | ||
| }, |
There was a problem hiding this comment.
When an inactive provider's expanded row reports a missing model, clicking its suggestion calls select_ai_model, which always updates self.ai_provider and set_active_model. For example, while Gemini is active, accepting a suggestion shown in an expanded OpenAI row pins that OpenAI model to Gemini and leaves OpenAI's missing pin unchanged, causing subsequent Gemini generation to use an invalid model; activate or explicitly update the provider represented by this row.
Useful? React with 👍 / 👎.
… provider A custom OpenAI-compatible endpoint may be a keyless local service — Ollama's `/v1` is the motivating case — but every guard demanded a stored credential, so the base URL the last round made reachable could only be used by inventing a dummy key. `requires_api_key` now answers that question in one place, the authorization header is omitted rather than sent empty, and the AI button, the command palette and the catalogue fetch all gate on `ai_credentials_ready`. Connection tests are tracked per provider. A single task slot dropped — and so cancelled — the test another row was still showing as in progress, leaving it on "Testing" with its button disabled until the window was reopened. `select_ai_model` takes the provider it is pinning for. A suggestion accepted from an expanded but inactive row went through `set_active_model`, which pinned that row's model to whichever provider happened to be active and left the row's own retired pin in place — so the next generation ran against a model the active provider does not have. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a76df0671
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| match scheme.to_ascii_lowercase().as_str() { | ||
| "https" => Ok(()), | ||
| // Ollama's `/v1` on the loopback interface is the main local case, and | ||
| // it does not serve TLS. | ||
| "http" if is_loopback_host(host) => Ok(()), | ||
| "http" => Err(BaseUrlError::InsecureScheme), |
There was a problem hiding this comment.
For an override such as http://localhost:80@evil.example/v1, host_without_port sees localhost and this branch accepts plain HTTP, while a standards-compliant URL parser treats evil.example as the destination and localhost:80 as userinfo. Generation can consequently send the configured bearer key and staged diff to a remote host without TLS despite the validator claiming HTTP is loopback-only; parse the URL structurally and reject userinfo before checking the actual host.
Useful? React with 👍 / 👎.
| this.update(cx, |this, cx| match outcome { | ||
| Ok(models) => { | ||
| this.ai_connection | ||
| .insert(provider, ConnectionState::Connected); |
There was a problem hiding this comment.
If the user starts a test and then removes the key or commits a different base URL before the request finishes, the old task remains alive and this callback unconditionally marks the provider connected and installs the old endpoint's catalogue. The settings window then reports the new configuration as verified using a result that describes different credentials or a different server; associate tests with a generation/configuration fingerprint and discard or cancel results when either changes.
AGENTS.md reference: AGENTS.md:L1-L1
Useful? React with 👍 / 👎.
| if has_key { | ||
| body = body.child(self.render_model_row(provider, tab_base, cx)); | ||
| } |
There was a problem hiding this comment.
Although requires_api_key now permits keyless custom endpoints, has_key remains false for them, so this gate hides the model row even after their /models request succeeds. Gateways such as the explicitly supported Ollama endpoint normally expose local model IDs rather than the selected provider's built-in default, leaving the user unable to choose a usable fetched model without adding a dummy credential or hand-editing settings.
Useful? React with 👍 / 👎.
| cx.update_global::<rgitui_settings::SettingsState, _>(|state, _cx| { | ||
| state.settings_mut().ai.commit_style = style.id().to_string(); | ||
| if let Err(error) = state.save() { | ||
| log::warn!("Failed to persist the commit style override: {}", error); |
There was a problem hiding this comment.
Choosing “Regenerate in a different style” is documented and modeled as a one-request override, but this assignment changes and persists the global preference. Every later generation therefore continues using the temporary style, and an already-open Settings window can later overwrite it from stale local state; pass the override into this generation instead of saving it to settings.ai.commit_style.
Useful? React with 👍 / 👎.
…rrides local `http://localhost:80@evil.example/v1` passed validation: everything before the `@` is userinfo, not the host, so reading `localhost` off the front let the loopback exemption wave through a plaintext request to a remote host carrying the API key and the staged diff. Userinfo is now rejected outright, with a message pointing at the API key field. Connection tests carry a generation id, and editing a key or committing a new base URL clears the provider's in-flight entry. A result describing the credentials or the server the user just replaced no longer reports the new configuration as verified. The settings row gates on whether a provider is usable, not on whether a key is stored, so a keyless gateway shows its model picker — the one place its local model ids can be selected — instead of onboarding. "Regenerate in a different style" is request-local again, as its own doc comment described. Writing it through to settings changed every later generation and raced an already-open Settings window holding the old value. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df0df7e3f2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for filename in PROJECT_CONTEXT_FILES { | ||
| let file_path = repo_path.join(filename); | ||
| if let Ok(contents) = std::fs::read_to_string(&file_path) { | ||
| if !contents.trim().is_empty() { |
There was a problem hiding this comment.
When a checkout contains README.md, CLAUDE.md, or AGENTS.md as a symlink to a file outside the repository, read_to_string follows it and places that external file in the AI prompt. Because project-context injection defaults to enabled, a cloned repository can therefore exfiltrate a local file (for example, by making README.md point at a credential file) the first time the user generates a commit message. Canonicalize each target and require it to remain beneath the canonical repository root before reading it.
Useful? React with 👍 / 👎.
| if let std::collections::btree_map::Entry::Vacant(e) = self.ai_catalog.entry(provider) { | ||
| let cached = catalog::read_cached(provider, &base_url); | ||
| let fresh = cached.as_ref().map(|cached| { | ||
| catalog::freshness(cached.fetched_at, catalog::now_unix()) | ||
| == catalog::CatalogFreshness::Fresh | ||
| }); | ||
| let (models, source) = catalog::resolve_catalog(provider, cached); |
There was a problem hiding this comment.
Opening a provider row calls read_cached directly from this UI update, and that function performs both std::fs::read_to_string and JSON deserialization synchronously. In particular, an unfiltered OpenRouter cache can be hundreds of kilobytes, so cold or slow storage and parsing can visibly stall the settings window. Load and parse the cache on the background executor, then apply the result on the UI thread with a stale-result guard, as required by the repository's imported threading rule.
AGENTS.md reference: AGENTS.md:L1-L1
Useful? React with 👍 / 👎.
| TextInputEvent::Changed(_) => { | ||
| this.invalidate_ai_connection(provider, cx); | ||
| this.schedule_secret_save(cx); |
There was a problem hiding this comment.
When a provider key is edited or removed, this only invalidates connection-test state; its current catalogue, cached catalogue, and any catalogue request using the previous key remain valid. Because /models results may be scoped to the credential, pasting a replacement key can leave the picker showing the old key's live list for the rest of the window session (or its fresh cache for up to 24 hours), and an old in-flight request can still install its result afterward. Supersede the provider's catalogue request and clear or refresh its catalogue when the key changes.
Useful? React with 👍 / 👎.
| for filename in PROJECT_CONTEXT_FILES { | ||
| let file_path = repo_path.join(filename); | ||
| if let Ok(contents) = std::fs::read_to_string(&file_path) { | ||
| if !contents.trim().is_empty() { | ||
| combined.push_str(&format!("=== {filename} ===\n{contents}\n\n")); |
There was a problem hiding this comment.
The advertised 50 KB context limit is applied only after every context file has been fully loaded and appended. A repository with an accidentally or maliciously huge README.md can therefore allocate hundreds of megabytes or exhaust process memory when generation starts, despite the final prompt being truncated. Read at most the remaining context budget from each file rather than using unbounded read_to_string.
Useful? React with 👍 / 👎.
… the UI thread Project-context injection is on by default, so `collect_project_context` runs against whatever a freshly cloned repository contains. It followed symlinks: shipping `README.md` as a link to a credential file put that file in the prompt the first time the user generated a commit message. Each target is now canonicalised and required to stay beneath the canonical repository root, the same rule `get_file_content` already applies to paths the model asks for. The 50 KB budget was also applied only after every file had been read in full, so one huge `README.md` allocated its whole size before being thrown away. Each file is now read up to the remaining budget, header and marker included, so the total can no longer overshoot — which also fixes the tail truncation cutting a newline-free file back to nothing. The settings window read and deserialised the model cache on the UI thread; an unfiltered OpenRouter catalogue is a few hundred KB of JSON. The bundled list renders immediately and the cache read, freshness check, request and cache write all happen on the background executor behind one stale-result guard. Editing a provider's key now invalidates its catalogue as well as its connection state: `/models` results can be scoped to the credential, so the old key's list was being shown for the rest of the session, its cache stayed fresh for 24 hours, and an in-flight request could still install its result. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 participant
Implements the AI subsystem overhaul plan in full, bar one item noted below.
The four bugs that were reachable out of the box
messages: []on iteration 1, which the Messages API rejects with a 400.use_toolsdefaults totrue, so that was the default path — this provider had never produced a commit message for anyone.gemini-2.0-flash, which appeared in no picker, andpill_groupmarks a pill selected on exact string equality. Three separate places disagreed about what the default was.pill_groupwas the one pill row in the file withoutflex_wrap(). Flex children default tomin-width: auto, so the row grew past its card and was sliced at the window edge. At the 880px default the default provider's own model list did not fit.get_file_content("../.env")was correctly rejected andget_file_content(".env")was accepted, uploadingDATABASE_URL=postgres://user:pass@…to the provider and echoing it back into the conversation for the remaining iterations.What changed
Correctness.
build_request_bodyis a pure function per provider, guarded by a test asserting none of them sends an empty conversation on the first iteration. String provider dispatch became anAiProviderenum owning endpoint, auth shape and default model, with unknown ids insettings.jsonreported throughload_warningsrather than surfacing after a click. Every Gemini response field is now optional and all parts are iterated, so a safety block, aMAX_TOKENSstop, a leading thought part and parallel function calls stop collapsing into one opaque parse error. Response bodies are capped at 8 MB, the Gemini key moved from the query string tox-goog-api-key, and the diff walker usesfrom_utf8_lossyso non-UTF-8 lines stop silently vanishing from what the model reasons about.Threading. The whole provider dispatch — HTTP, JSON parsing,
gitspawns,read_dirwalks — moved onto the background executor, with tool progress arriving over anmpscchannel that a small foreground task drains. Every request has a 60s deadline and itsTaskis held, so a stalled provider no longer leaves the spinner running until the app is restarted. Every event carries aGenerationId { sequence, repo_path }and routes by repo path rather thanactive_tab, so a message generated for one tab cannot land in another's commit box. The cooldown is stamped on completion rather than dispatch, and reported as info without clearing an in-flight generation's spinner.Security.
get_file_contentdenies.git/, a credentials denylist, git-ignored paths and non-UTF-8 content before reading anything, and re-checks the canonicalised path so an in-repo symlink cannot reach.git/config.Per-provider credentials. One keychain slot per provider, migrated from the single shared slot, with per-provider model pins — switching provider no longer destroys the previous provider's key or its model choice, and "connected" is no longer asserted for a provider the app has no credential for.
Features. Live model catalogue with stale-while-revalidate caching, a bundled fallback and a generation guard, labelled so "three weeks old" is distinguishable from "shipped with the app". OpenRouter, via the existing OpenAI-compatible path widened with an endpoint struct.
base_url_overridefor that family, validated, warning inline which host the key will reach.UI. A provider accordion replacing six flat cards, each row owning its key field, connection status and model pin, with a connection test that can tell a working key from a typo. The AI button states its own reason when it cannot be used, stays reachable when the fix is one click away, and finally shows the shortcut that was registered all along. Tool progress is routed to the panel chip, with cancel, regenerate-with-style, and undo of an AI overwrite. Errors stop auto-dismissing at 3s and can carry an action. The Save button is gone — text inputs flush on Enter and on blur, so the same value no longer persists or vanishes depending on how it arrived.
tab_indexnow exists on the settings page, which previously had none across 4,215 lines.Removals.
last_result/last_errorhad no callers;uuidwas one call site whose id was never transmitted;http_clientwas never referenced by path;fuzzy_scoreis now one shared implementation rather than three.Migration
CURRENT_SETTINGS_VERSION2 → 3. The v2→v3 step remaps known-retired model ids to successors, andmigrate_legacy_secretspromotes the sharedai/defaultkey into the active provider's slot.ai/defaultis deliberately left in place so a downgrade still finds its key; it can be deleted a release later. Every newAiSettingsfield is#[serde(default)], so v2 files load unchanged.Verification
cargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings, andcargo test --workspaceall pass — 1665 tests, up from ~1180. New coverage is pure and display-free per theCLAUDE.mdconvention: request-body shape per provider, endpoint and attribution-header resolution,base_url_overridevalidation, Gemini parsing against the four shapes that used to produce one error, catalogue parsing against a trimmed real OpenRouter fixture (negative-price BYOK row, nulltop_provider.context_length, records missingreasoning/benchmarks), filter/freshness/pinned-model classification, the tool denylist and output budget, and the shared AI guard.Two things to know before merging
Streaming is not included. Everything else in that plan item — the overwrite guard, undo, read-only editors during generation — is. Streaming needs per-provider SSE parsing including tool-call deltas across three different wire formats, with no way to exercise it offline; the
ToolCallStarted→ chip re-route covers the feedback gap in the meantime. Happy to take it on separately.The default model ids are unverified. They come from the plan's research, which flags them⚠️ and says to confirm against a live API; I could not make real requests. The structural fix means a wrong id is now visible and correctable — the picker classifies it as
Missingand offers the closest match — rather than silently rendering an unselected row, but each default still wants one real request to confirm before release.🤖 Generated with Claude Code