feat(desktop): aghub side panel revamp#141
Conversation
… sqlx Replace file-based inference_providers.json with an SQLite database, using sqlx 0.8 with the migrate feature for schema management. - Add sqlx (sqlite + runtime-tokio + migrate) and tokio deps - Add migrations/0001_create_inference_providers.sql - Rewrite InferenceProviderStore to use per-call SqliteConnection - Bridge sync trait to async sqlx via block_in_place + Handle fallback - Replace InferenceProviderError::Json with ::Database (sqlx/migrate) - Fix aghub-api error mapping for the renamed variant Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add OpenCode provider CRUD backed by auth.json and opencode.json, plus matching/sync against stored inference providers. Split inference provider stable names from display names for UI editing.
Add Codex config.toml provider CRUD using Responses API providers only, wire it through API routes, and expose Codex provider management in the desktop inference page.
- Add PlayIcon enable button to both ClaudeOfficialRow and ClaudeProviderRow - Show tooltip with reason when provider is already active - Update tooltip copy for unmanaged OpenCode credentials - Add claudeProviderAlreadyActive translation keys
Remove is_active column from agent_provider_bindings. Active state should be derived from agent config file, not stored in DB. - Add derive_active_binding() to Claude adapter - Mark config.toml providers as External (non-deletable) - Add External variant to AgentProviderSource - Update frontend tooltip for External providers - Fix migration to safely drop is_active column - All 61 inference tests pass
# Conflicts: # Cargo.lock # Cargo.toml # crates/api/Cargo.toml # crates/api/src/lib.rs # crates/desktop/src-tauri/src/commands/server.rs # crates/desktop/src/App.tsx # crates/desktop/src/generated/dto/index.ts # crates/desktop/src/lib/api.ts
…page - Replace navigator.clipboard.writeText with static import from @tauri-apps/plugin-clipboard-manager - Aligns with main branch clipboard migration (4246c00)
Replace the ternary `trimmedApiKey ? trimmedApiKey : null` with the shorter `trimmedApiKey || null` form. Behavior is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Introduces the new `/` landing page that frames the product as "your agent hub": a grid of installed agents (sorted ready first, then disabled, then alphabetical), each card showing skill / MCP counts and an Open button. Includes an "Add agent" placeholder card and a quick-actions row to the market and agent settings. The page is unreachable until the route table and sidebar wire it up in a later commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ents tabs Introduces `/agents/:agentId` as the agent-scoped workspace. The header shows the agent's icon, name, scope hint, and a primary "Open config folder" button that uses Tauri's revealItemInDir to surface the agent's actual config file in Finder/Explorer. Tabs reuse the existing global pages as embedded views so list behaviour stays consistent. The Model tab renders the inference provider page for Claude/Codex/OpenCode and shows a "managed by the agent" placeholder for the rest. Adds `lib/agent-config-paths.ts` which resolves a best-known filesystem path per agent: hardcoded `~`-relative paths for the 13 agents whose locations are well-known (`~/.claude.json`, `~/.codex/config.toml`, `~/.config/opencode/opencode.json`, etc.) falling back to `skills_paths.global_write` for the rest. Returns null for agents we don't recognise so the button is hidden. The route is unreachable until the App route table wires it up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…s page Adds a single search box (intended to live in the sidebar) that fuzzy-matches across the user's installed agents, skills, MCP servers, sub-agents, and the skills.sh library catalogue. Results are grouped per category and capped per group in the popover. The popover supports keyboard navigation (arrow keys + enter on a highlighted row jumps to it). Pressing enter without a highlighted row, or clicking the always-visible "View all results" footer, navigates to the new `/search?q=...` page. The full-page view shows the same groups without per-group caps; sections with more than 8 rows collapse to a "Show N more" toggle so a 100+ skill list does not dominate the screen. `useGlobalSearch` extracts the matching logic so both the popover and the full page share one source of truth. Library results are debounced (250ms) and capped at 5 in the popover, 20 on the page. The search box is unreachable until a later commit mounts it in the sidebar. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Introduces the unified `/market` discovery hub. The page is a single route with four tabs, switched via `?tab=`: - **Skills.sh** mounts the existing search-driven catalogue page. - **MCP Marketplace** shows a 12-card hardcoded grid of popular servers (filesystem, github, postgres, slack, puppeteer, memory, brave-search, linear, notion, sentry, stripe, google-drive) as a teaser. Each "Add manually" button jumps to /mcp. A preview callout at the top makes it clear this is not yet a live registry. - **Claude Code Plugins** shows a coming-soon placeholder with a link to the official plugin docs. - **Import from GitHub** mounts the existing import wizard panel. The page is unreachable until the App route table wires it up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… hub
Pivots the product from a flat resource manager into an agent-first
hub. The sidebar now reads top-down as Search → Home/Market →
All Resources → All Agents → Projects → Settings, with a fixed
search at the top, a single scrollable region in the middle, and
Settings pinned at the bottom.
Sidebar structure:
- Search box mounts the new <GlobalSearch /> component.
- Two top-level destinations (Home, Market) sit above the first
separator.
- "All Resources" exposes the existing global views — Skills,
MCP Servers, Sub-agents, Inference Providers — for power users
who want a cross-agent radar.
- "All Agents" lists installed agents only (filtered by
availability.is_available, disabled agents pushed to the bottom).
Long lists collapse to 5 with a "Show N more" toggle that always
keeps the active agent pinned visible.
- Project list now follows the same section-label spacing and no
longer has a stray mt-4 leftover from the pre-separator layout.
Routes:
- New: /, /agents/:agentId/:rest*, /search, /market, /market/search.
- Cross-agent views remain reachable: /skills, /mcp, /sub-agents,
/inference-providers all stay alive (no longer redirect to /).
- /library and /skills-sh redirect to /market and /market/search;
in-app links in skill-detail, menu, home, and skills-sh pages
now point at /market.
Store:
- SidebarItemId shrinks from five resource ids to two static ids
("home" | "market"). v6→v7 wipes legacy preferences; v7→v8 swaps
"library" for "market". CURRENT_VERSION bumped to 8.
- use-sidebar-navigation drops the visibility/order mutation
surface — the static items are no longer user-reorderable.
Other cleanup:
- Deletes the unused settings/sidebar-panel.tsx and removes its
link from appearance-panel.
- Adds 30+ i18n keys across en/zh-Hans/zh-Hant covering Home,
Market, search, agent statuses, model-not-configurable hints,
and the open-config-folder action.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Acts on feedback that "what users want to manage are resources, not agents". The previous agent-first IA forced everyone through an agent sidebar entry and a per-agent detail page; that hurt power users who think in cross-agent resource terms and required re-implementing similar list UIs four times. Reshapes navigation around three principles: 1. Resources are the noun, agents are an attribute. 2. Cross-agent views are the default; agent scoping is a filter layered on top, not a separate view. 3. There is one list page per resource type, parameterised by ?agent=<id>. No per-agent UI components. Sidebar: - Drops the "All Agents" section entirely. - Renames "All Resources" to "Resources" (Skills / MCP / Sub-agents). - Lifts Inference Providers into its own section since it is per-agent configuration, not a cross-agent resource. Resource pages: - Skills, MCP, and Sub-agents now read a `?agent=` query param via nuqs and apply a client-side filter. - A new <AgentFilterChip /> renders at the top of each page when a filter is active and offers an ✕ to clear. Sidebar filter persistence: - New module-level "sticky" store (`useStickyAgentFilter`) remembers the last agent the user filtered by, so navigating away to Inference Providers / Market / Settings and back to Skills/MCP/ Sub-agents preserves the filter. The chip's clear button resets both the URL and the sticky store. - Sidebar resource links call `withAgent(href, sticky)` so clicking Skills → MCP carries the filter forward without re-selecting. Home: - Agent cards now navigate to `/skills?agent=<id>` (default first resource view) instead of the deleted agent detail page. - Each card gains a folder-open icon button that calls revealItemInDir on the agent's resolved config path; this used to live in the now-deleted agent header. Routing: - /agents/:agentId/* routes are deleted. The two remaining route declarations redirect any deep links to /skills?agent=<id> so external bookmarks and deep links continue to work. - Global search hrefs follow the new shape: agent → /skills?agent=X skill → /skills?agent=X&skill=Y mcp → /mcp?agent=X&server=Y sub-agent → /sub-agents?agent=X Wouter / nuqs interaction fix: - nuqs's React adapter only listens to popstate and its own emitter, so the sidebar's useQueryState wouldn't react to wouter's pushState navigations. The sidebar now reads ?agent= via wouter's reactive useSearch() hook, then mirrors it into the sticky store via an effect on resource pages. Deletes: - src/pages/agent/* (the agent detail page and its tab wrappers). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After rebasing onto upstream's plugin discovery work, wire the new PluginsPage into the agent-hub sidebar and Market layout. Sidebar: - Add a "Claude Code Plugins" entry to the Resources section, alongside Skills, MCP Servers, and Sub-agents. Plugins do not carry the agent filter forward (Claude is the only agent that consumes them) so the resource link map gained a per-entry `carriesAgentFilter` flag and only the three agent-scoped resources opt in. Market: - Replace the "coming soon" placeholder in the Claude Code Plugins tab with the real PluginsPage component. The tab and the new /cc-plugins route both render the same UI so users can reach plugin management from either the discovery hub or the resources section. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pulls the search + category filter + table + counts UI out of PluginMarketDialog into a new <PluginMarketContent> component that both the dialog and the Market page's "Claude Code Plugins" tab consume. The Market tab now renders the real install browser instead of mounting the management page (which was a quick-fix from the prior commit). This restores the conceptual split: - /market?tab=claude-plugins → discover and install (browse the marketplace, filter by category, install with one click). - /cc-plugins → manage installed plugins (toggle enable, configure scopes, uninstall). PluginMarketDialog becomes a ~30-line wrapper around the shared content and now resets its inline filters when reopened by bumping a key on close, so users don't see stale search/category state on their next visit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Several layered improvements to how Claude Code plugins surface in the new Market hub. Press-lag fixes on the plugin market table: - Drop the 200ms `transition-colors` on the install button so press feedback is immediate; the animation was the source of perceived lag, not actual work. - Memoize each row body into <PluginMarketRowCells>. Previously the whole table re-rendered every time installStateById / transientPluginsById changed — for ~100 plugin rows that was enough to feel sluggish. Now only the row whose state actually changed re-renders. - Stabilize handleInstall via a ref-backed wrapper so the memoized rows don't all bust on every parent state update. Layout: PluginMarketTable now takes a `variant` prop. The legacy modal variant keeps its `min-h-[16rem] max-h-[52vh]` capped chrome with rounded background; the new "page" variant fills the parent column and drops the rounded box that was clashing visually with the surrounding tab. Lazy-load heavy Market tabs (PluginMarketContent and ImportGithubSkillPanel) so users who land on Market via the default Skills.sh tab don't pay the cost of plugin / GitHub-import JS until they switch tabs. Each lazy tab gets its own Suspense fallback. The "Install from market" button on /cc-plugins now navigates to /market?tab=claude-plugins instead of opening PluginMarketDialog. The dialog component still exists for callers that want a modal, but the management page itself just routes to the discovery hub — keeps the conceptual split (manage vs discover) consistent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the in-pane <ListSearchHeader> on the three resource pages (Skills, MCP Servers, Sub-agents) with a single full-width toolbar that lives at the top of each page. The 280px list pane was getting visually cramped trying to host filter + search + multi-select + add + refresh in one row; lifting them out gives the search field proper room and makes the agent filter affordance impossible to miss. New <ResourcePageToolbar> component lays out: [agent filter] [search field, flex-1] [trailing icon buttons] The agent filter is a Dropdown that doubles as the active-state indicator: when no filter is set it shows a `[⊕ All agents ▾]` ghost button; when active it switches to a secondary-tone button showing the agent's icon + name + an inline ✕ to clear without opening the menu. Same component, two visual states — users always see one place to change the filter and always see what's currently filtered. The previous standalone <AgentFilterChip> row above the search bar is removed and the component file deleted; its UX intent is now absorbed into the toolbar. i18n: adds agentFilterAllAgents / agentFilterIdleLabel / agentFilterActiveLabel / agentFilterChangeLabel / agentFilterShowAll across en, zh-Hans, zh-Hant. The older filteringByAgent and clearAgentFilter keys are kept for the time being. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR redesigns desktop hub navigation by simplifying the sidebar to home+market, introducing global search with fuzzy matching, adding agent-scoped filtering across resource pages, creating home and market dashboard pages, refactoring plugin marketplace into a market tabbed interface, and updating all three locale files with new Hub navigation strings. ChangesHub navigation and user experience redesign
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (8)
crates/inference/src/credentials.rs-19-24 (1)
19-24:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClarify concurrency guarantees or adjust type design.
The documentation warns that "OS keyring backends may not support concurrent writes reliably" and suggests callers "serialize access themselves," but
NativeCredentialStorederivesCopyandClone, making it easy to use concurrently without synchronization. This creates an inconsistency:
- If concurrent access is unsafe, the type should not be
Copy + Clone, or should require&mut selffor mutations.- If the
keyringcrate handles concurrency safely, the warning should be removed or clarified.🛡️ Possible fixes
Option 1: If keyring handles concurrency, remove the warning:
-/// OS keyring backends may not support concurrent writes reliably. Callers -/// sharing this store across threads should serialize access themselves. #[derive(Debug, Clone, Copy, Default)] pub struct NativeCredentialStore;Option 2: If serialization is needed, remove
Copyand useArc<Mutex<>>:-/// OS keyring backends may not support concurrent writes reliably. Callers -/// sharing this store across threads should serialize access themselves. -#[derive(Debug, Clone, Copy, Default)] -pub struct NativeCredentialStore; +/// OS keyring backends may not support concurrent writes reliably. +/// Use Arc<Mutex<NativeCredentialStore>> when sharing across threads. +#[derive(Debug, Default)] +pub struct NativeCredentialStore { + _private: (), +}Option 3: Change methods to require
&mut self:impl CredentialStore for NativeCredentialStore { - fn get_api_key(&self, provider_id: &str) -> Result<Option<String>> { + fn get_api_key(&mut self, provider_id: &str) -> Result<Option<String>> {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/inference/src/credentials.rs` around lines 19 - 24, The docs warn about unsafe concurrent writes but the struct derives Copy and Clone, which encourages unsynchronized sharing; fix by making NativeCredentialStore non-Copy/non-Clone and updating the docs: remove the Copy and Clone derives from NativeCredentialStore, keep Debug (and Default only if still meaningful), and change the comment to explicitly state callers must wrap the store in synchronization primitives (e.g., Arc<Mutex<...>>) when sharing across threads; reference the NativeCredentialStore type in the comment so it's clear which API requires external serialization.crates/desktop/src/pages/settings/skills.tsx-116-119 (1)
116-119:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReset multi-select state when the agent filter changes.
selectedKeysis preserved across filters, so skills that are no longer visible can still count as selected and keep the bulk-action bar out of sync with the filtered list. Clearing selection and leaving multi-select mode here avoids that stale state.💡 Minimal fix
const handleAgentFilterChange = (next: string | null) => { setAgentFilter(next); setStickyAgentFilter(next); + setSelectedKeys(new Set()); + setIsMultiSelectMode(false); };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/desktop/src/pages/settings/skills.tsx` around lines 116 - 119, handleAgentFilterChange currently only updates setAgentFilter and setStickyAgentFilter but leaves multi-select state and selectedKeys intact, causing stale selections after filter changes; update handleAgentFilterChange to also clear selection and exit multi-select mode by calling the selection state updaters (e.g., setSelectedKeys(new Set()) to clear selectedKeys and setIsMultiSelect(false) or setSelectionMode(false) to leave multi-select), so filtered-out skills can't remain selected and the bulk-action bar stays in sync.crates/desktop/src/pages/settings/mcp-servers.tsx-154-157 (1)
154-157:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReset multi-select state when the agent filter changes.
selectedKeyssurvives the filter change, so hidden MCP groups can remain "selected" and the floating-bar count drifts from the visible/deleteable set. Clearing selection and exiting multi-select here keeps bulk actions aligned with the filtered list.💡 Minimal fix
const handleAgentFilterChange = (next: string | null) => { setAgentFilter(next); setStickyAgentFilter(next); + setSelectedKeys(new Set()); + setIsMultiSelectMode(false); };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/desktop/src/pages/settings/mcp-servers.tsx` around lines 154 - 157, When the agent filter changes via handleAgentFilterChange, clear the multi-select state so hidden MCP groups aren't still considered selected: after calling setAgentFilter(next) and setStickyAgentFilter(next) also reset the selection (e.g. clear selectedKeys) and exit multi-select mode (e.g. call the function that toggles/clears multi-select) so bulk-action counts reflect only visible items; update handleAgentFilterChange to invoke the selection-clear and multi-select-exit helpers (referencing selectedKeys, setSelectedKeys or the component's multi-select toggle) immediately after the two existing setters.crates/desktop/src/lib/store/migrations/v7-to-v8.ts-4-6 (1)
4-6:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDuplicate migration logic detected.
Both
migrateV6ToV7andmigrateV7ToV8perform the identical operation: setting"sidebarItems"toDEFAULT_SIDEBAR_ITEMS. Running two separate migration steps that do the same thing is redundant and suggests a potential migration design issue.If the sidebar items value needs to be reset to defaults during this migration window, consider consolidating these steps into a single migration or clarifying why both are necessary.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/desktop/src/lib/store/migrations/v7-to-v8.ts` around lines 4 - 6, Both migrateV6ToV7 and migrateV7ToV8 currently duplicate the same logic (await store.set("sidebarItems", DEFAULT_SIDEBAR_ITEMS)); remove the redundancy by consolidating the operation into a single migration step: either delete the duplicate function body in migrateV7ToV8 or move the sidebar reset into the earlier migrateV6ToV7 and make migrateV7ToV8 a no-op that returns immediately; ensure the remaining migration is idempotent and clearly commented so future reviewers see the intent.crates/api/src/bin/export-dto.rs-15-27 (1)
15-27:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winVerify line width compliance.
The import block contains several long type names that may exceed the 80-character line width limit specified in the coding guidelines. Please verify that these lines comply with the maximum line width requirement for Rust code.
As per coding guidelines, "Maintain maximum line width of 80 characters in Rust code".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/api/src/bin/export-dto.rs` around lines 15 - 27, The multi-item import list (including symbols like AgentProviderCredentialDto, AgentProviderMatchedInferenceProviderResponse, AgentProviderModelResponse, AgentProviderResponse, AgentProviderSourceDto, ClaudeProviderStateResponse, CodexProfileResponse, CodexProviderStateResponse, CreateAgentProviderRequest, CreateInferenceProviderRequest, InferenceProviderFormatDto, InferenceProviderPasswordResponse, InferenceProviderPresetResponse, InferenceProviderResponse, UpdateAgentProviderRequest, UpdateClaudeProviderRequest, UpdateCodexActiveProfileRequest, UpdateCodexProfileProviderRequest, UpdateInferenceProviderRequest) exceeds the 80-character line width; reflow the use/inference::{ ... } block so no line is longer than 80 chars—prefer one imported type per line (or small logical groups) with a trailing comma and proper indentation, then run rustfmt to confirm compliance.crates/desktop/src/pages/inference-providers.tsx-418-455 (1)
418-455:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
handleFetchsilently drops empty rows the user just typed into.The
baseline = value.filter((model) => model.name.trim())step is meant to discard the placeholder "empty model" entry, but it also throws away any genuinely-empty rows the user added via "Add model" and was about to fill in. After "Fetch models" runs, those rows vanish without warning. Either keep all rows except the synthetic placeholder (e.g. tag the placeholder with a known id), or merge fetched results intovaluewithout filtering empties.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/desktop/src/pages/inference-providers.tsx` around lines 418 - 455, handleFetch currently drops any user-added empty rows because baseline = value.filter((model) => model.name.trim()) removes all empty-name entries; change it to preserve user-empty rows and only exclude the synthetic placeholder entry. Update createProviderModelFormValue (or the placeholder-creation logic) to mark the placeholder (e.g., set a unique id or flag like isPlaceholder), then in handleFetch replace the name.trim() filter with a check that excludes only that placeholder (e.g., model.isPlaceholder or model.id === PLACEHOLDER_ID) so fetched additions are merged with existing rows without deleting user-empty rows; ensure onChange([...baseline, ...additions]) then uses the preserved baseline.crates/desktop/src/pages/inference-providers/claude-panel.tsx-78-82 (1)
78-82:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winWrong success toast on create.
This is the create-provider dialog, but on success it shows
t("claudeProviderUpdated"). Use a creation-specific message (e.g.claudeProviderCreated) so users get accurate feedback and the i18n keys stay aligned with the action.🐛 Suggested change
onSuccess: async () => { - toast.success(t("claudeProviderUpdated")); + toast.success(t("claudeProviderCreated")); onClose(); },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/desktop/src/pages/inference-providers/claude-panel.tsx` around lines 78 - 82, The success toast in the create-provider flow is using the wrong i18n key; in the onSuccess handler of the creation mutation in claude-panel.tsx (the callback that calls toast.success(...) and onClose()), change the i18n key from t("claudeProviderUpdated") to the creation-specific key t("claudeProviderCreated") so the message reflects creation and stays aligned with i18n.crates/desktop/src/pages/inference-providers.tsx-110-152 (1)
110-152:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a timeout / abort to
fetchProviderModels.A user-supplied
apiBaseUrlis fetched with no timeout and noAbortSignal. If the host is unreachable, slow, or a load balancer holds the connection open, the "Fetch models" button stays in pending state indefinitely (and the button can't really be cancelled by closing the dialog because the promise is still resolving). Wire anAbortControllerwith a sane timeout (e.g. 15–30 s) so the editor degrades gracefully.🛡️ Suggested change
- const response = await fetch(`${trimmedBase}/models`, { - method: "GET", - headers, - }); + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 30_000); + let response: Response; + try { + response = await fetch(`${trimmedBase}/models`, { + method: "GET", + headers, + signal: controller.signal, + }); + } finally { + clearTimeout(timeoutId); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/desktop/src/pages/inference-providers.tsx` around lines 110 - 152, The fetchProviderModels function currently performs fetch without cancellation; add an AbortController with a timeout (e.g. 15–30s) inside fetchProviderModels, pass controller.signal into the fetch call, and set a timer to call controller.abort() after the timeout; ensure you clear the timer on success and throw or rethrow a clear timeout/aborted Error so callers can handle it. Locate the function fetchProviderModels and update the fetch invocation to include the AbortSignal, create the controller and timer before calling fetch, and clean up the timer/controller in all success and error paths.
🧹 Nitpick comments (13)
crates/desktop/src/lib/agent-config-paths.ts (1)
38-39: ⚡ Quick winHarden fallback path access with optional chaining.
This avoids runtime risk if
skills_pathsis ever missing on a partially-populatedAgentInfo.Suggested refactor
- if (agent.skills_paths.global_write) { - return agent.skills_paths.global_write; + if (agent.skills_paths?.global_write) { + return agent.skills_paths.global_write; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/desktop/src/lib/agent-config-paths.ts` around lines 38 - 39, The code accesses agent.skills_paths.global_write directly which can throw if skills_paths is undefined; update the check and return to use optional chaining (e.g., test agent.skills_paths?.global_write) and handle undefined safely (return the value only if present, otherwise fall through to the existing fallback logic) so that AgentInfo objects missing skills_paths won’t cause runtime errors when evaluating global_write.crates/inference/src/opencode/schema.rs (1)
48-54: 💤 Low valueConsider using is_empty methods directly in skip_serializing_if.
The helper functions are thin wrappers around
is_empty(). You could potentially use the method directly in the serde attribute:#[serde(default, skip_serializing_if = "Map::is_empty")]However, the current explicit functions are clearer and easier to understand, so this is a minor style consideration.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/inference/src/opencode/schema.rs` around lines 48 - 54, The two tiny wrapper functions is_json_map_empty and is_model_map_empty simply call .is_empty(); replace their use in serde attributes with the method references directly (e.g., use Map::is_empty for the JSON map field and BTreeMap::is_empty for the model map field in skip_serializing_if) or remove the wrappers if they are only used for serde to reduce indirection; update any serde attributes referencing is_json_map_empty/is_model_map_empty to reference the inherent method names instead.crates/desktop/src/components/plugin-market/market-table.tsx (1)
45-59: 💤 Low valueConsider documenting formatter stability requirement.
The
PluginMarketRowCellscomponent is memoized for performance, but it receivescompactFormatteras a prop. If the parent component recreates the formatter on each render, the memoization will be bypassed. Consider adding a JSDoc comment documenting thatcompactFormattershould be stable (created withuseMemoin the parent).📝 Documentation suggestion
interface RowCellsProps { plugin: CCPluginMarketResponse; installState: TableInstallState; + /** + * Number formatter for install counts. Should be a stable reference + * (e.g., created with useMemo) to avoid unnecessary re-renders. + */ compactFormatter: Intl.NumberFormat; onInstall: (pluginId: string) => void; labels: {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/desktop/src/components/plugin-market/market-table.tsx` around lines 45 - 59, The memoized PluginMarketRowCells component accepts compactFormatter via the RowCellsProps interface; document that compactFormatter must be a stable reference (e.g., created with useMemo in the parent) so memoization isn't invalidated by a new Intl.NumberFormat each render. Add a JSDoc comment above RowCellsProps or the PluginMarketRowCells declaration stating that compactFormatter must be stable and suggesting useMemo in the parent to create it, and mention that recreating it every render will bypass memoization.crates/inference/src/error.rs (1)
10-16: ⚡ Quick winPreserve source error information for better debugging.
The
DatabaseandKeyringvariants convert errors toString, which loses the original error type and stack trace. Storing the source error (or using#[source]) would preserve the error chain and make debugging easier.♻️ Proposed refactor to preserve error sources
/// SQLite database error. #[error("database error: {0}")] - Database(String), + Database(#[source] sqlx::Error), /// Platform keyring error. #[error("keyring error: {0}")] - Keyring(String), + Keyring(#[source] keyring::Error),Then update the
Fromimplementations:impl From<keyring::Error> for InferenceProviderError { fn from(error: keyring::Error) -> Self { - Self::Keyring(error.to_string()) + Self::Keyring(error) } } impl From<sqlx::Error> for InferenceProviderError { fn from(error: sqlx::Error) -> Self { - Self::Database(error.to_string()) + Self::Database(error) } } impl From<sqlx::migrate::MigrateError> for InferenceProviderError { fn from(error: sqlx::migrate::MigrateError) -> Self { - Self::Database(error.to_string()) + Self::Database(error.into()) } }This preserves the original error for debugging while still providing a good display message via
thiserror.Also applies to: 90-106
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/inference/src/error.rs` around lines 10 - 16, Change the Database and Keyring variants in the error enum to carry the original error as a source (e.g., Database(#[source] Box<dyn std::error::Error + Send + Sync>) and Keyring(#[source] Box<dyn std::error::Error + Send + Sync>)) instead of converting to String, and update the corresponding From implementations that produce these variants to Box the incoming error (or use .into()) so the original error type and chain are preserved for debugging while keeping the thiserror display messages intact; ensure any matches/uses of Database and Keyring are adjusted to account for the boxed/source error type.crates/desktop/src/pages/market.tsx (1)
343-347: 💤 Low valueConsider implementing onDone callback for GitHub import panel.
Line 345 passes an empty function
onDone={() => {}}toImportGithubSkillPanel. If the import panel is expected to notify the parent on completion (e.g., to refresh data or navigate), this callback should be implemented. If no action is needed, consider adding a comment explaining why.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/desktop/src/pages/market.tsx` around lines 343 - 347, Replace the empty onDone handler passed to ImportGithubSkillPanel with a real callback that performs the parent action on completion (for example, call the refresh function that reloads marketplace data or run navigation logic); locate the ImportGithubSkillPanel usage inside the activeTab === "github" branch and update the onDone prop to invoke the appropriate parent method (e.g., refreshMarketplace(), fetchSkills(), or navigateTo(...)) or, if no action is required, add a short inline comment explaining why the handler is intentionally a no-op to make intent explicit.crates/inference/src/model.rs (1)
148-159: ⚡ Quick winConsider consistent redaction format in Debug impl.
Line 156 uses
.map(|_| "[redacted]")which will printSome("[redacted]")when the field is present, butNonewhen absent. This is inconsistent with theCreateInferenceProviderDebug impl (line 121) which always shows"[redacted]"as a direct reference.For consistency and clarity, consider matching the pattern from lines 113-124:
♻️ Suggested consistent redaction pattern
impl fmt::Debug for UpdateInferenceProvider { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let redacted = "[redacted]"; f.debug_struct("UpdateInferenceProvider") .field("name", &self.name) .field("display_name", &self.display_name) .field("format", &self.format) .field("api_base_url", &self.api_base_url) .field("models", &self.models) - .field("api_key", &self.api_key.as_ref().map(|_| "[redacted]")) + .field("api_key", &self.api_key.as_ref().map(|_| &redacted)) .finish() } }This will print
api_key: Some("[redacted]")when present andapi_key: Nonewhen absent, making the presence/absence information explicit while keeping the value redacted.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/inference/src/model.rs` around lines 148 - 159, The Debug impl for UpdateInferenceProvider should use the same redaction expression as CreateInferenceProvider so the api_key field's formatting is consistent; update the .field("api_key", ...) entry in the fmt::Debug impl for UpdateInferenceProvider to map the Option<String> to an Option<&str> redaction (e.g., use self.api_key.as_deref().map(|_| "[redacted]")) so the debug output shows Some("[redacted]") when present and None when absent, matching the other provider's Debug behavior.crates/desktop/src/components/app-sidebar.tsx (1)
67-79: ⚡ Quick winUse the URL agent for link building on the first render.
carriedAgentcomes fromstickyAgent, but that store is only updated after the effect runs. On a direct load or back-nav to/skills?agent=..., the resource links can render with the previous filter for one cycle. DerivingcarriedAgentfromonResource ? urlAgent : stickyAgentavoids that lag even if you keep the effect for persistence.♻️ Suggested adjustment
- const carriedAgent = stickyAgent; + const carriedAgent = onResource ? urlAgent : stickyAgent;As per coding guidelines, "Use
useMemoand event handlers for state synchronization instead ofuseEffect."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/desktop/src/components/app-sidebar.tsx` around lines 67 - 79, carriedAgent should use the current URL value on initial render instead of waiting for the useEffect to update stickyAgent; change the carriedAgent derivation to use onResource ? urlAgent : stickyAgent (preferably wrapped in useMemo) so links render with urlAgent on direct loads/back-nav, while keeping the existing useEffect for persistence; update references to carriedAgent accordingly and remove any reliance on the pre-effect stickyAgent value.crates/desktop/src/pages/inference-providers/codex-panel.tsx (1)
109-109: ⚡ Quick winUse
cnutility for className concatenation.Multiple instances throughout the file use direct className strings. According to coding guidelines, you should use the
cnutility from@/lib/utilsfor className concatenation instead of string templates.Examples:
- Line 109:
className="sm:max-w-[440px]"- Line 117:
className="grid gap-4 p-4"- Line 231:
className="grid gap-3 border-t border-border py-3..."As per coding guidelines, "Use the
cnutility from@/lib/utilsfor className concatenation instead of string templates".Also applies to: 117-117, 231-231, 321-321, 333-333, 549-549
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/desktop/src/pages/inference-providers/codex-panel.tsx` at line 109, The file uses raw className string literals in several JSX elements (e.g., Modal.Dialog, container divs and footer sections) instead of the cn utility; import cn from '@/lib/utils' if not present and replace the literal className attributes (examples: Modal.Dialog className="sm:max-w-[440px]", the grid/padding className at the element referenced around the "grid gap-4 p-4" text, the footer element with "grid gap-3 border-t border-border py-3...", and the other instances near the lines mentioned) with cn(...) calls so class concatenation follows the project guideline; ensure you update all occurrences listed (lines ~109, ~117, ~231, ~321, ~333, ~549) and preserve the exact class strings as arguments to cn.crates/desktop/src/pages/inference-providers.tsx (2)
191-225: 💤 Low value
dangerouslySetInnerHTMLhere is acceptable but worth pinning down.Static analysis flags lines 196 and 221, but the SVG strings come from a build‑time, vendored
@lobehub/icons-static-svgimport (?raw), so they're not user-controlled and the XSS vector is effectively closed at the bundler boundary. Worth a brief comment at the injection sites stating the trust assumption so a future contributor doesn't replace the source with a runtime-fetched string. No code change needed if you accept this tradeoff.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/desktop/src/pages/inference-providers.tsx` around lines 191 - 225, Static-analysis warns about using dangerouslySetInnerHTML in ProviderIcon and PresetLogo; add a short inline comment at both injection sites (inside ProviderIcon and PresetLogo, near the dangerouslySetInnerHTML usage) stating that the SVG strings originate from a build-time, vendored `@lobehub/icons-static-svg` import (raw) and are not user-controlled, documenting the trust assumption so future contributors don't replace them with runtime-fetched content; no functional code changes required, just the explanatory comments referencing ProviderIcon, PRESET_LOGO_MAP and PresetLogo.
1334-1352: 💤 Low valueRevealed/copied API keys live in plain memory — keep the existing flow but minimize exposure.
Both
revealedKeyand the locally-fetchedpassword.api_keyinhandleCopyKeyare ordinary JS strings, so they sit in V8 heap until GC. That's typical for desktop UIs, but two small hardenings keep the blast radius small without changing the UX:
- Auto-clear
revealedKeyafter a short timeout (e.g. 30 s) when revealed, similar to common password-manager UIs.- Don't store the fetched key from
handleCopyKeypast theawait writeTextcall (already the case, good).Also applies to: 1362-1380
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/desktop/src/pages/inference-providers.tsx` around lines 1334 - 1352, When a provider API key is revealed, automatically clear the in-memory revealed value after a short timeout (e.g., 30s) to minimize exposure: update the state flow that uses revealedKey (and its setter setRevealedKey) so that when you set a revealed key you also start a timer that calls setRevealedKey(null) after the timeout and ensure the timer is cleared if the key is hidden earlier; in handleCopyKey, continue to avoid persisting the fetched password.api_key beyond the await navigator.clipboard.writeText call by not assigning it to any longer-lived state or variables and by immediately dropping any local variable holding it (or scoping it so it goes out of scope) after writeText completes.crates/inference/src/codex/files.rs (1)
57-70: ⚖️ Poor tradeoffMake home‑directory failure a typed error, not an
io::Error.
home_dir_errorconstructs anio::Errorand wraps it inInferenceProviderError::Io. Callers that want to distinguish "no home dir" from a real disk I/O failure (for clearer user messaging or different retry/UX) can't easily do that. A dedicated variant such asInferenceProviderError::MissingHomeDir(or a more descriptiveInvalidAgentProviderConfigpayload) would compose better with downstream error reporting.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/inference/src/codex/files.rs` around lines 57 - 70, The helper home_dir_error currently builds an std::io::Error and wraps it in InferenceProviderError::Io; instead add a dedicated error variant (e.g. InferenceProviderError::MissingHomeDir or InferenceProviderError::InvalidAgentProviderConfig) and return that from home_dir_error(), then update default_global_config_path to call home_dir_error() as before; change the enum InferenceProviderError declaration to include the new variant and adjust any match sites that expect Io so callers can distinguish "no home dir" from real I/O errors (refer to functions default_global_config_path and home_dir_error and the InferenceProviderError enum).crates/desktop/src/pages/inference-providers/claude-panel.tsx (1)
394-400: 💤 Low valueUnused prop reads as dead code.
onEditInferenceProvideris declared on the panel signature but never used (_:ignores the entire props object). If the prop is intentionally part of the cross-panel contract for symmetry withOpenCodeInferenceProviderPanel/CodexInferenceProviderPanel, prefer destructuring with a leading-underscore name so intent is explicit and TS still validates the call site:♻️ Suggested change
-export function ClaudeInferenceProviderPanel(_: { - onEditInferenceProvider: (providerName: string) => void; -}) { +export function ClaudeInferenceProviderPanel( + _props: { onEditInferenceProvider: (providerName: string) => void }, +) {Or wire the prop up if Claude rows should also be able to deep-link into the inference-provider editor.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/desktop/src/pages/inference-providers/claude-panel.tsx` around lines 394 - 400, The component ClaudeInferenceProviderPanel declares a prop onEditInferenceProvider but currently ignores the props via the unnamed parameter `_` which makes the prop read as dead code; update the function signature to destructure the prop while keeping a leading-underscore local name (e.g. onEditInferenceProvider: _onEditInferenceProvider) so TypeScript still enforces the contract and intent is explicit, or alternatively wire the prop into the UI by calling onEditInferenceProvider from the row action handlers inside ClaudeInferenceProviderPanel where similar panels use it; locate the function ClaudeInferenceProviderPanel and adjust the parameter destructuring or usage accordingly.crates/desktop/src/pages/inference-providers/opencode-panel.tsx (1)
233-238: ⚡ Quick winDrop the form‑reset
useEffect— the dialog already remounts.The parent renders this dialog with
{providerDialog?.type === "edit" && (...)}andonCloseflips that back tonull, so the component unmounts on close and remounts on the next "edit" click. InitialuseState(provider.name)already gives you a fresh form each open, making this effect redundant. Removing it also aligns with the project rule of preferringuseMemo/ event handlers overuseEffectfor state synchronization.♻️ Suggested change
- const [name, setName] = useState(provider.name); - const [apiKey, setApiKey] = useState(""); - const [nameError, setNameError] = useState<string | null>(null); - - useEffect(() => { - if (!isOpen) return; - setName(provider.name); - setApiKey(""); - setNameError(null); - }, [isOpen, provider.id, provider.name]); + const [name, setName] = useState(provider.name); + const [apiKey, setApiKey] = useState(""); + const [nameError, setNameError] = useState<string | null>(null);As per coding guidelines: "Use
useMemoand event handlers for state synchronization instead ofuseEffect".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/desktop/src/pages/inference-providers/opencode-panel.tsx` around lines 233 - 238, Remove the redundant useEffect that resets form state on open: delete the useEffect block that depends on [isOpen, provider.id, provider.name] which calls setName(provider.name), setApiKey(""), and setNameError(null). The dialog already unmounts/remounts (parent toggles providerDialog) and initial useState(provider.name) provides a fresh form, so rely on that instead of syncing state in useEffect; keep handlers and state initializers (setName, setApiKey, setNameError) as-is.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f5d46f0b-9491-4d45-ae99-57995b8a85d0
⛔ Files ignored due to path filters (23)
Cargo.lockis excluded by!**/*.lockcrates/desktop/bun.lockis excluded by!**/*.lockcrates/desktop/src/generated/dto/AgentProviderCredentialDto.tsis excluded by!**/generated/**crates/desktop/src/generated/dto/AgentProviderMatchedInferenceProviderResponse.tsis excluded by!**/generated/**crates/desktop/src/generated/dto/AgentProviderModelResponse.tsis excluded by!**/generated/**crates/desktop/src/generated/dto/AgentProviderResponse.tsis excluded by!**/generated/**crates/desktop/src/generated/dto/AgentProviderSourceDto.tsis excluded by!**/generated/**crates/desktop/src/generated/dto/ClaudeProviderStateResponse.tsis excluded by!**/generated/**crates/desktop/src/generated/dto/CodexProfileResponse.tsis excluded by!**/generated/**crates/desktop/src/generated/dto/CodexProviderStateResponse.tsis excluded by!**/generated/**crates/desktop/src/generated/dto/CreateAgentProviderRequest.tsis excluded by!**/generated/**crates/desktop/src/generated/dto/CreateInferenceProviderRequest.tsis excluded by!**/generated/**crates/desktop/src/generated/dto/InferenceProviderFormatDto.tsis excluded by!**/generated/**crates/desktop/src/generated/dto/InferenceProviderPasswordResponse.tsis excluded by!**/generated/**crates/desktop/src/generated/dto/InferenceProviderPresetResponse.tsis excluded by!**/generated/**crates/desktop/src/generated/dto/InferenceProviderResponse.tsis excluded by!**/generated/**crates/desktop/src/generated/dto/UpdateAgentProviderRequest.tsis excluded by!**/generated/**crates/desktop/src/generated/dto/UpdateClaudeProviderRequest.tsis excluded by!**/generated/**crates/desktop/src/generated/dto/UpdateCodexActiveProfileRequest.tsis excluded by!**/generated/**crates/desktop/src/generated/dto/UpdateCodexProfileProviderRequest.tsis excluded by!**/generated/**crates/desktop/src/generated/dto/UpdateInferenceProviderRequest.tsis excluded by!**/generated/**crates/desktop/src/generated/dto/index.tsis excluded by!**/generated/**crates/inference/inference_providers.dbis excluded by!**/*.db
📒 Files selected for processing (84)
Cargo.tomlcrates/api/Cargo.tomlcrates/api/src/bin/export-dto.rscrates/api/src/dto/data/inference_provider_presets.jsoncrates/api/src/dto/inference.rscrates/api/src/dto/mod.rscrates/api/src/error.rscrates/api/src/lib.rscrates/api/src/main.rscrates/api/src/routes/inference.rscrates/api/src/routes/mod.rscrates/api/src/state.rscrates/desktop/package.jsoncrates/desktop/src-tauri/capabilities/default.jsoncrates/desktop/src-tauri/src/commands/server.rscrates/desktop/src/App.tsxcrates/desktop/src/components/app-sidebar.tsxcrates/desktop/src/components/global-search.tsxcrates/desktop/src/components/plugin-market-content.tsxcrates/desktop/src/components/plugin-market-dialog.tsxcrates/desktop/src/components/plugin-market/market-table.tsxcrates/desktop/src/components/project-list.tsxcrates/desktop/src/components/resource-page-toolbar.tsxcrates/desktop/src/components/skill-detail.tsxcrates/desktop/src/hooks/use-global-search.tscrates/desktop/src/hooks/use-sidebar-navigation.tscrates/desktop/src/hooks/use-sticky-agent-filter.tscrates/desktop/src/lib/agent-config-paths.tscrates/desktop/src/lib/api.tscrates/desktop/src/lib/locales/en.tscrates/desktop/src/lib/locales/zh-Hans.tscrates/desktop/src/lib/locales/zh-Hant.tscrates/desktop/src/lib/menu.tscrates/desktop/src/lib/sidebar-navigation.tscrates/desktop/src/lib/store/migrations/index.tscrates/desktop/src/lib/store/migrations/v6-to-v7.tscrates/desktop/src/lib/store/migrations/v7-to-v8.tscrates/desktop/src/lib/store/types.tscrates/desktop/src/pages/home.tsxcrates/desktop/src/pages/inference-providers.tsxcrates/desktop/src/pages/inference-providers/claude-panel.tsxcrates/desktop/src/pages/inference-providers/codex-panel.tsxcrates/desktop/src/pages/inference-providers/opencode-panel.tsxcrates/desktop/src/pages/inference-providers/provider-selection.tscrates/desktop/src/pages/market.tsxcrates/desktop/src/pages/plugins/index.tsxcrates/desktop/src/pages/search.tsxcrates/desktop/src/pages/settings/appearance-panel.tsxcrates/desktop/src/pages/settings/mcp-servers.tsxcrates/desktop/src/pages/settings/sidebar-panel.tsxcrates/desktop/src/pages/settings/skills.tsxcrates/desktop/src/pages/settings/sub-agents.tsxcrates/desktop/src/pages/skills-sh/index.tsxcrates/desktop/src/pages/skills-sh/search.tsxcrates/desktop/src/requests/inference-providers.tscrates/desktop/src/requests/keys.tscrates/inference/Cargo.tomlcrates/inference/migrations/0001_create_inference_providers.sqlcrates/inference/migrations/0002_create_inference_models.sqlcrates/inference/migrations/0003_add_masked_api_key.sqlcrates/inference/migrations/0004_add_display_name.sqlcrates/inference/migrations/0005_create_agent_provider_bindings.sqlcrates/inference/migrations/0006_drop_binding_is_active.sqlcrates/inference/migrations/0007_scope_agent_provider_binding_ids.sqlcrates/inference/src/agent.rscrates/inference/src/claude/files.rscrates/inference/src/claude/mod.rscrates/inference/src/claude/tests.rscrates/inference/src/codex/files.rscrates/inference/src/codex/mapping.rscrates/inference/src/codex/mod.rscrates/inference/src/codex/tests.rscrates/inference/src/credentials.rscrates/inference/src/error.rscrates/inference/src/lib.rscrates/inference/src/model.rscrates/inference/src/opencode/files.rscrates/inference/src/opencode/mapping.rscrates/inference/src/opencode/mod.rscrates/inference/src/opencode/schema.rscrates/inference/src/opencode/tests.rscrates/inference/src/store.rscrates/json/Cargo.tomlcrates/json/src/lib.rs
💤 Files with no reviewable changes (2)
- crates/desktop/src/pages/settings/sidebar-panel.tsx
- crates/desktop/src/pages/settings/appearance-panel.tsx
| [ | ||
| { | ||
| "id": "openai", | ||
| "name": "OpenAI", | ||
| "api_base_url": "https://api.openai.com/v1", | ||
| "format": "openai_responses", | ||
| "models": ["gpt-5.5", "gpt-5.4", "gpt-5.4-mini"], | ||
| "logo": "OpenAI", | ||
| "homepage": "https://platform.openai.com/api-keys", | ||
| "description": "Official OpenAI API." | ||
| }, | ||
| { | ||
| "id": "anthropic", | ||
| "name": "Anthropic", | ||
| "api_base_url": "https://api.anthropic.com", | ||
| "format": "anthropic", | ||
| "models": ["claude-opus-4-7", "claude-sonnet-4-6", "claude-haiku-4-5"], | ||
| "logo": "Anthropic", | ||
| "homepage": "https://console.anthropic.com/settings/keys", | ||
| "description": "Claude models via the Anthropic Messages API." | ||
| }, | ||
| { | ||
| "id": "openrouter", | ||
| "name": "OpenRouter", | ||
| "api_base_url": "https://openrouter.ai/api/v1", | ||
| "format": "openai_completions", | ||
| "models": [ | ||
| "openai/gpt-5.5", | ||
| "anthropic/claude-sonnet-4.6", | ||
| "meta-llama/llama-4-maverick" | ||
| ], | ||
| "logo": "OpenRouter", | ||
| "homepage": "https://openrouter.ai/keys", | ||
| "description": "Unified gateway for 100+ models." | ||
| }, | ||
| { | ||
| "id": "groq", | ||
| "name": "Groq", | ||
| "api_base_url": "https://api.groq.com/openai/v1", | ||
| "format": "openai_completions", | ||
| "models": [ | ||
| "llama-3.3-70b-versatile", | ||
| "openai/gpt-oss-120b", | ||
| "llama-3.1-8b-instant" | ||
| ], | ||
| "logo": "Groq", | ||
| "homepage": "https://console.groq.com/keys", | ||
| "description": "Low-latency inference on LPU hardware." | ||
| }, | ||
| { | ||
| "id": "mistral", | ||
| "name": "Mistral", | ||
| "api_base_url": "https://api.mistral.ai/v1", | ||
| "format": "openai_completions", | ||
| "models": [ | ||
| "mistral-large-latest", | ||
| "mistral-small-latest", | ||
| "codestral-latest" | ||
| ], | ||
| "logo": "Mistral", | ||
| "homepage": "https://console.mistral.ai/api-keys", | ||
| "description": "Mistral AI's hosted models." | ||
| }, | ||
| { | ||
| "id": "together", | ||
| "name": "Together", | ||
| "api_base_url": "https://api.together.xyz/v1", | ||
| "format": "openai_completions", | ||
| "models": [ | ||
| "meta-llama/Llama-3.3-70B-Instruct-Turbo", | ||
| "Qwen/Qwen3-Coder-480B-A35B-Instruct-FP8", | ||
| "deepseek-ai/DeepSeek-V4-Pro" | ||
| ], | ||
| "logo": "Together", | ||
| "homepage": "https://api.together.xyz/settings/api-keys", | ||
| "description": "Open-source model hosting." | ||
| }, | ||
| { | ||
| "id": "deepseek", | ||
| "name": "DeepSeek", | ||
| "api_base_url": "https://api.deepseek.com/v1", | ||
| "format": "openai_completions", | ||
| "models": ["deepseek-v4-flash", "deepseek-v4-pro"], | ||
| "logo": "DeepSeek", | ||
| "homepage": "https://platform.deepseek.com/api_keys", | ||
| "description": "DeepSeek's hosted Chat and Reasoner models." | ||
| } | ||
| ] |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What are the currently available OpenAI GPT models as of May 2026?
💡 Result:
As of May 10, 2026, OpenAI’s available GPT(-family) models for the API include the following model IDs listed in OpenAI’s official model catalog. Frontier (GPT-5 / GPT-5.x) - gpt-5.5 [1] - gpt-5.5 pro [1] - gpt-5.4 [1] - gpt-5.4 pro [1] - gpt-5.4 mini [1] - gpt-5.4 nano [1] - gpt-5.3-chat-latest (GPT-5.3 Chat) [2] - gpt-5.3-codex [3] - gpt-5.2 [1] - gpt-5.2 pro [4] - gpt-5.1 [1] - gpt-5 [5] - gpt-5 mini [1] - gpt-5 nano [1] Multimodal / flagship non-reasoning GPT - gpt-4o [1][6] - gpt-4o mini [1][7] - gpt-4.1 [8] Audio / speech GPT(-family) - gpt-realtime [1] - gpt-realtime-1.5 [1] - gpt-realtime-mini [1] - gpt-realtime-2 [1] - gpt-audio-1 [1] - gpt-audio [9] - gpt-audio-mini [10] - gpt-4o-audio-preview (GPT-4o Audio) [11] Other GPT(-family) specialized variants visible in OpenAI’s catalog - gpt-4o-mini-search-preview [12] Note: OpenAI also supports dynamically listing “currently available models” via the API endpoint GET /v1/models, which is the most reliable way to confirm the exact set for your account/region at a specific date. [13][14] If you want, I can format the above into a clean machine-readable list (JSON/YAML) or provide the exact curl snippet to enumerate models from your API key. [13][14]
Citations:
- 1: https://developers.openai.com/api/docs/models/all
- 2: https://developers.openai.com/api/docs/models/gpt-5.3-chat-latest
- 3: https://developers.openai.com/api/docs/models/gpt-5.3-codex
- 4: https://developers.openai.com/api/docs/models/gpt-5.2-pro
- 5: https://developers.openai.com/api/docs/models/gpt-5
- 6: https://platform.openai.com/docs/models/gpt-4o
- 7: https://developers.openai.com/api/docs/models/gpt-4o-mini
- 8: https://platform.openai.com/docs/models/gpt-4.1/
- 9: https://developers.openai.com/api/docs/models/gpt-audio
- 10: https://platform.openai.com/docs/models/gpt-audio-mini
- 11: https://developers.openai.com/api/docs/models/gpt-4o-audio-preview
- 12: https://developers.openai.com/api/docs/models/gpt-4o-mini-search-preview
- 13: https://developers.openai.com/api/reference/resources/models/methods/list/
- 14: https://platform.openai.com/docs/api-reference/models/list?lang=node.js
🌐 Web query:
What are the currently available Anthropic Claude models as of May 2026?
💡 Result:
As of 2026-05-10, Anthropic’s currently available (active) Claude models (Claude API model IDs) are: 1) Claude Opus family - claude-opus-4-7 [1] - claude-opus-4-6 [1] - claude-opus-4-5-20251101 [2] - claude-opus-4-1-20250805 [2] 2) Claude Sonnet family - claude-sonnet-4-6 [1][2] - claude-sonnet-4-5-20250929 [2] - (Deprecated but still available as of May 2026) - claude-sonnet-4-20250514 (deprecated; retirement scheduled for June 15, 2026) [2] 3) Claude Haiku - claude-haiku-4-5-20251001 [1][2] Also present, but not “currently available” for use (they are listed as retired): - claude-3-7-sonnet-20250219 (retired Oct 28, 2025) [2] - claude-3-5-haiku-20241022 (retired Dec 19, 2025) [2] - claude-3-haiku-20240307 (retired Feb 19, 2026) [2] Notes: - Anthropic’s “Models overview” page lists the currently available models and their Claude API IDs. [1] - The “Model deprecations” page provides the active/deprecated/retired status and retirement dates. [2] - You can also programmatically discover what models are available from the Models API (GET /v1/models). [3]
Citations:
- 1: https://docs.anthropic.com/en/docs/about-claude/models/all-models
- 2: https://docs.anthropic.com/en/docs/resources/model-deprecations
- 3: https://console.anthropic.com/docs/en/api/models/list
🌐 Web query:
What model names does OpenRouter support as of May 2026?
💡 Result:
OpenRouter supports model names (model “slugs”) that you pass as the model field in API requests. These slugs are dynamically listed by OpenRouter in its Models API; each returned item includes a unique model identifier id intended for API requests (example format: "google/gemini-2.5-pro-preview").[1] There is no single static, human-maintained “complete list” page that can be safely treated as the definitive set “as of May 2026” because the catalog changes frequently; instead, OpenRouter’s authoritative source is the live GET https://openrouter.ai/api/v1/models listing endpoint, which returns the full catalog at request time.[2][3] How to retrieve the complete (May 2026) list yourself (authoritative): - Call the OpenRouter Models API listing endpoint: - GET https://openrouter.ai/api/v1/models (optionally filter by modality via output_modalities, and filter by capability via supported_parameters).[3][4] - Use the id field from each returned model object as the model name you can send to the model parameter.[1] Example model-name formats supported (concrete slugs shown in OpenRouter docs): - anthropic/claude-haiku-4.5[5] - meta-llama/llama-3.3-70b-instruct[5] - google/gemini-3.1-pro-preview[5] - openai/gpt-5.2-pro[5] - x-ai/grok-3-mini[5] - z-ai/glm-5[5] If you specifically want the set of slugs “as of May 10, 2026” (not just how to obtain them), tell me whether you mean: 1) all modalities (text/image/audio/embeddings), or 2) only text LLMs, and whether you want active models only (exclude “inactive models”). Then I can extract the model slugs from the catalog in that same category using OpenRouter’s live listing semantics described in the docs.[3][4]
Citations:
- 1: https://openrouter.ai/docs/guides/overview/models.mdx
- 2: https://openrouter.ai/models
- 3: https://openrouter.ai/docs/changelog/2026/5/6
- 4: https://dynamicsinfo.com/docs/guides/overview/models
- 5: https://openrouter.ai/)
Update model names to match current provider API documentation.
The preset model names have naming convention mismatches and incomplete model IDs:
- OpenAI:
gpt-5.4-minishould use space separator:gpt-5.4 mini - Anthropic: Model IDs require date suffixes (e.g.,
claude-haiku-4-5-20251001notclaude-haiku-4-5); also addclaude-opus-4-6and other available variants - OpenRouter: Verify
meta-llama/llama-4-maverickexists; examples in OpenRouter docs showllama-3.3-70bvariants instead
Test with actual provider APIs (GET /v1/models endpoints) to confirm availability before users encounter failures.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/api/src/dto/data/inference_provider_presets.json` around lines 1 - 88,
Update the provider preset model names to match current provider docs: for the
"openai" entry update the "models" array to use the corrected token (e.g.,
change "gpt-5.4-mini" to "gpt-5.4 mini"); for the "anthropic" entry replace
short IDs like "claude-haiku-4-5" with full dated IDs (e.g.,
"claude-haiku-4-5-20251001") and add missing variants such as "claude-opus-4-6"
in the "models" array; for the "openrouter" entry validate each model string
(e.g., confirm or replace "meta-llama/llama-4-maverick" with an actually listed
model such as llama-3.3-70b variants) and correct any vendor-prefixed IDs in its
"models" array; after edits, confirm availability by calling each provider's GET
/v1/models and update entries to the exact model IDs returned.
| #[get("/inference/providers/<name>/password")] | ||
| pub fn get_inference_provider_password( | ||
| state: &State<InferenceProviderState>, | ||
| name: &str, | ||
| ) -> ApiResult<InferenceProviderPasswordResponse> { | ||
| let store = store(state); | ||
| let provider = find_by_name(&store, name)?; | ||
| let api_key = store | ||
| .get_api_key(&provider.id) | ||
| .map_err(ApiError::from)? | ||
| .ok_or_else(|| { | ||
| ApiError::new( | ||
| Status::NotFound, | ||
| format!( | ||
| "inference provider '{}' has no stored API key", | ||
| provider.display_name | ||
| ), | ||
| "RESOURCE_NOT_FOUND", | ||
| ) | ||
| })?; | ||
|
|
||
| Ok(Json(InferenceProviderPasswordResponse { | ||
| name: provider.name, | ||
| api_key, | ||
| })) |
There was a problem hiding this comment.
Avoid returning raw API keys from a GET route.
This handler serializes the full stored secret back to the client. Even for a desktop app, raw keys in API responses are easy to leak through logs, crash reports, devtools, or any untrusted renderer/plugin with access to the local API. Prefer one-way credential updates and return only masked metadata here.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/api/src/routes/inference.rs` around lines 468 - 492, The GET handler
get_inference_provider_password currently returns the full stored API key (via
store.get_api_key and InferenceProviderPasswordResponse) which risks leaking
secrets; change this to never serialize the raw secret: modify
get_inference_provider_password to fetch the key only to assert existence and
instead return masked metadata (e.g., provider.name, a boolean has_api_key or
masked value like "*****…", and optional last_updated), and implement a separate
POST/PUT endpoint for setting/updating the API key; ensure store.get_api_key is
only used to check presence and that the response type
(InferenceProviderPasswordResponse) is adjusted accordingly so raw api_key is
not included in responses.
| <button | ||
| type="button" | ||
| onPointerDown={handleClear} | ||
| aria-label={t("clearAgentFilter")} | ||
| className="rounded-full p-0.5 text-muted transition-colors hover:bg-default hover:text-foreground focus:outline-none focus:ring-2 focus:ring-accent/40" | ||
| > | ||
| <XMarkIcon className="size-3.5" /> | ||
| </button> |
There was a problem hiding this comment.
Don't nest the clear control inside the trigger button.
<Button> already renders an interactive control, so the inner native <button> creates invalid button-in-button markup. It also only listens to onPointerDown, which means keyboard activation won't clear the filter reliably. Split the clear action into a sibling control or make the outer trigger handle clearing/opening differently.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/desktop/src/components/resource-page-toolbar.tsx` around lines 131 -
138, The clear control is an inner native <button> nested inside the outer
<Button>, causing invalid button-in-button markup and only handling pointer
events via onPointerDown; change it to a separate sibling control and use a
keyboard-accessible handler. Remove the inner <button> that renders the
XMarkIcon and instead render a sibling interactive element (e.g., an IconButton
or plain <button>) next to the outer <Button>, wire its onClick to handleClear
(not onPointerDown), preserve aria-label={t("clearAgentFilter")} and the classes
(rounded-full …), and ensure you reference the existing handleClear function and
XMarkIcon so keyboard activation and semantics are correct.
| async function updateInferenceProviderModels( | ||
| providerName: string, | ||
| models: (current: string[]) => string[], | ||
| ) { | ||
| const provider = await getInferenceProviderByName(providerName); | ||
| return client | ||
| .put(`inference/providers/${encodeURIComponent(provider.name)}`, { | ||
| json: { | ||
| name: null, | ||
| display_name: null, | ||
| format: null, | ||
| api_base_url: null, | ||
| api_key: null, | ||
| models: models(provider.models), | ||
| } satisfies UpdateInferenceProviderRequest, | ||
| }) | ||
| .json<InferenceProviderResponse>(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm whether a per-model endpoint exists on the API side and how the
# update handler treats null fields in UpdateInferenceProviderRequest.
rg -nP --type=rust -C3 'inference/providers' crates/api
rg -nP --type=rust -C5 'UpdateInferenceProvider(Request|\b)' cratesRepository: AkaraChen/aghub
Length of output: 14250
Read‑modify‑write race on concurrent models edits.
updateInferenceProviderModels fetches the current provider state, then PUTs the updated models array back. If another client adds or removes a model between the GET and PUT, that concurrent change will be silently lost—the PUT replaces the entire models list with what this client observed at GET time.
Example: Client A and B both read models: ["gpt-4"]. Client A adds "gpt-3.5" and PUTs ["gpt-4", "gpt-3.5"]. Client B adds "claude-3" and PUTs ["gpt-4", "claude-3"], overwriting A's addition.
To prevent this:
- Prefer dedicated
POST/DELETE /inference/providers/:name/models/:modelendpoints for individual model mutations, or - Add an
If-Matchheader or version field for optimistic locking.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/desktop/src/lib/api.ts` around lines 112 - 129,
updateInferenceProviderModels currently does a read-modify-write via
getInferenceProviderByName and client.put, which allows lost concurrent updates;
modify this to use optimistic locking or per-model endpoints: either (A) change
the call to include an If-Match header using a version/etag from
getInferenceProviderByName (e.g., use provider.etag or provider.version) and
handle 412 responses by retrying or surfacing an error, ensuring the client.put
to `inference/providers/${encodeURIComponent(provider.name)}` uses that header,
or (B) replace the bulk PUT usage with dedicated endpoints such as POST
/inference/providers/:name/models/:model and DELETE
/inference/providers/:name/models/:model and call those from
updateInferenceProviderModels (or add new helper functions) so each mutation is
atomic; locate symbols updateInferenceProviderModels,
getInferenceProviderByName, client.put, UpdateInferenceProviderRequest and
InferenceProviderResponse to implement the chosen fix.
| let replacement_model = all_providers | ||
| .iter() | ||
| .find(|binding| binding.id == provider_id) | ||
| .and_then(|binding| { | ||
| let current_is_valid = | ||
| current_model.as_ref().is_some_and(|model_id| { | ||
| binding.models.iter().any(|m| m.id == *model_id) | ||
| }); | ||
| if current_is_valid { | ||
| None | ||
| } else { | ||
| binding.models.first().map(|model| model.id.clone()) | ||
| } | ||
| }); | ||
|
|
||
| if profile_id == DEFAULT_PROFILE_ID { | ||
| let table = config.as_table_mut(); | ||
| if provider_id == mapping::OPENAI_PROVIDER_ID { | ||
| table.remove("model_provider"); | ||
| } else { | ||
| table["model_provider"] = value(provider_id.clone()); | ||
| } | ||
| } else { | ||
| let profile = profile_table_mut(&mut config, &profile_id)?; | ||
| profile["model_provider"] = value(provider_id.clone()); | ||
| } | ||
| if let Some(model_id) = replacement_model { | ||
| if profile_id == DEFAULT_PROFILE_ID { | ||
| config["model"] = value(model_id); | ||
| } else { | ||
| let profile = profile_table_mut(&mut config, &profile_id)?; | ||
| profile["model"] = value(model_id); | ||
| } | ||
| } |
There was a problem hiding this comment.
Switching providers can leave a stale model override behind.
If the current model is invalid for the new provider and binding.models.first() is None, Lines 215-222 never touch the existing model. That happens for the built-in OpenAI binding because its model list is empty, so clearing/switching providers can strand a provider-specific model in config.toml. Clear the profile/default model when there is no compatible replacement.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/inference/src/codex/mod.rs` around lines 189 - 222, The code computes
replacement_model but leaves an invalid per-profile/default "model" when
binding.models.first() is None (e.g., built-in OpenAI has empty models); update
the logic so that when the current model is not valid for the new provider and
replacement_model is None you explicitly clear the "model" override: detect the
invalid case (same check as current_is_valid used in the closure comparing
current_model and binding.models), then if profile_id == DEFAULT_PROFILE_ID
remove the "model" key from config (like table.remove("model")) otherwise get
the profile via profile_table_mut(&mut config, &profile_id)? and remove the
"model" key from that profile; use the existing symbols replacement_model,
current_model, mapping::OPENAI_PROVIDER_ID, DEFAULT_PROFILE_ID, and
profile_table_mut to locate where to add this clearing step.
| pub fn add_provider( | ||
| &self, | ||
| provider_id: &str, | ||
| provider: &InferenceProvider, | ||
| api_key: &str, | ||
| ) -> Result<AgentProviderBinding> { | ||
| mapping::ensure_api_key(api_key)?; | ||
| let provider_id = mapping::clean_provider_id(provider_id)?; | ||
| let credential = AgentProviderCredential::AgentStore { | ||
| id: Some(provider_id.clone()), | ||
| }; | ||
| let binding = AgentProviderBinding::from_inventory( | ||
| provider_id.clone(), | ||
| provider, | ||
| credential, | ||
| AgentProviderSource::Custom, | ||
| )?; | ||
|
|
||
| let mut state = self.load_providers()?; | ||
| state.providers.retain(|item| item.id != provider_id); | ||
| state.providers.push(binding.clone()); | ||
| self.set_api_auth(&provider_id, api_key)?; | ||
| self.save_providers(&state)?; | ||
|
|
||
| Ok(binding) | ||
| } | ||
|
|
||
| /// Add a provider using a slug derived from its stable key. | ||
| pub fn add_inventory_provider( | ||
| &self, | ||
| provider: &InferenceProvider, | ||
| api_key: &str, | ||
| ) -> Result<AgentProviderBinding> { | ||
| let provider_id = mapping::provider_id_from_name(&provider.name); | ||
| self.add_provider(&provider_id, provider, api_key) | ||
| } | ||
|
|
||
| /// Update an existing OpenCode provider's display name and/or API key. | ||
| pub fn update_provider( | ||
| &self, | ||
| provider_id: &str, | ||
| name: Option<&str>, | ||
| api_key: Option<&str>, | ||
| ) -> Result<AgentProviderBinding> { | ||
| let provider_id = mapping::clean_provider_id(provider_id)?; | ||
| let current = self | ||
| .load_providers()? | ||
| .providers | ||
| .into_iter() | ||
| .find(|provider| provider.id == provider_id) | ||
| .ok_or_else(|| { | ||
| crate::error::InferenceProviderError::NotFound( | ||
| provider_id.clone(), | ||
| ) | ||
| })?; | ||
|
|
||
| let name = name.map(mapping::clean_provider_name).transpose()?; | ||
| if let Some(api_key) = api_key { | ||
| mapping::ensure_api_key(api_key)?; | ||
| } | ||
|
|
||
| if let Some(api_key) = api_key { | ||
| self.set_api_auth(&provider_id, api_key)?; | ||
| } | ||
|
|
||
| if let Some(name) = name { | ||
| let mut config = files::read_config(&self.config_path)?; | ||
| config.provider.entry(provider_id.clone()).or_default().name = | ||
| Some(name); | ||
| files::write_config(&self.config_path, config)?; | ||
| } | ||
|
|
||
| Ok(self | ||
| .load_providers()? | ||
| .providers | ||
| .into_iter() | ||
| .find(|provider| provider.id == provider_id) | ||
| .unwrap_or(current)) | ||
| } | ||
|
|
||
| /// Read an API key visible to OpenCode for a provider. | ||
| pub fn api_key(&self, provider_id: &str) -> Result<Option<String>> { | ||
| let provider_id = mapping::clean_provider_id(provider_id)?; | ||
| let auth = files::read_auth_values(&self.auth_path)?; | ||
| if let Some(api_key) = | ||
| auth.get(&provider_id).and_then(auth_entry_api_key) | ||
| { | ||
| return Ok(Some(api_key.to_string())); | ||
| } | ||
|
|
||
| let config = files::read_config(&self.config_path)?; | ||
| let Some(api_key) = config | ||
| .provider | ||
| .get(&provider_id) | ||
| .and_then(|provider| provider.options.get("apiKey")) | ||
| .and_then(Value::as_str) | ||
| else { | ||
| return Ok(None); | ||
| }; | ||
|
|
||
| if let Some(env_name) = parse_env_var_ref(api_key) { | ||
| return Ok(std::env::var(env_name).ok()); | ||
| } | ||
|
|
||
| Ok(Some(api_key.to_string())) | ||
| } | ||
|
|
||
| /// Remove a provider definition and matching OpenCode auth entry. | ||
| pub fn remove_provider( | ||
| &self, | ||
| provider_id: &str, | ||
| ) -> Result<AgentProviderBinding> { | ||
| let provider_id = mapping::clean_provider_id(provider_id)?; | ||
| let mut state = self.load_providers()?; | ||
| let removed = state | ||
| .providers | ||
| .iter() | ||
| .find(|provider| provider.id == provider_id) | ||
| .cloned() | ||
| .ok_or_else(|| { | ||
| crate::error::InferenceProviderError::NotFound( | ||
| provider_id.clone(), | ||
| ) | ||
| })?; | ||
|
|
||
| state | ||
| .providers | ||
| .retain(|provider| provider.id != provider_id); | ||
| self.save_providers(&state)?; | ||
|
|
||
| let mut auth = files::read_auth_values(&self.auth_path)?; | ||
| if auth.remove(&provider_id).is_some() { | ||
| files::write_auth_values(&self.auth_path, &auth)?; | ||
| } | ||
|
|
||
| Ok(removed) | ||
| } |
There was a problem hiding this comment.
Make config/auth updates atomic (or add rollback) across provider mutations.
Lines 97-102, 140-149, and 207-212 perform multi-file writes without transactional protection. A failure between writes can leave opencode.json and auth.json out of sync.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/inference/src/opencode/mod.rs` around lines 79 - 215, The multi-file
updates in add_provider, update_provider, and remove_provider can leave
opencode.json and auth.json inconsistent if one write fails; change these flows
to perform atomic or transactional writes by preparing both updated in-memory
representations (provider state and auth/config) and then persisting them using
a two-phase safe write (write each to a temp file then atomically rename) or by
performing the second write inside a rollback block that restores the first file
on failure; update functions referenced: add_provider, update_provider,
remove_provider, set_api_auth, save_providers, files::write_config, and
files::write_auth_values to use the temp-then-rename pattern or a rollback
restore to guarantee both files are consistent.
# Conflicts: # Cargo.lock # Cargo.toml # crates/desktop/bun.lock # crates/desktop/package.json # crates/desktop/src/App.tsx # crates/desktop/src/components/plugin-market-dialog.tsx # crates/desktop/src/components/plugin-market/market-table.tsx # crates/desktop/src/generated/dto/index.ts # crates/desktop/src/lib/api.ts # crates/desktop/src/lib/locales/en.ts # crates/desktop/src/lib/locales/zh-Hans.ts # crates/desktop/src/lib/locales/zh-Hant.ts # crates/desktop/src/lib/sidebar-navigation.ts # crates/desktop/src/lib/store/types.ts # crates/desktop/src/pages/inference-providers.tsx # crates/desktop/src/pages/inference-providers/opencode-panel.tsx # crates/inference/src/agent.rs # crates/inference/src/codex/files.rs # crates/inference/src/codex/mapping.rs # crates/inference/src/codex/mod.rs # crates/inference/src/codex/tests.rs # crates/inference/src/opencode/files.rs # crates/inference/src/opencode/mod.rs # crates/inference/src/opencode/tests.rs
- Resolve 7 conflicts: take main for the inference backend/page and the plugin marketplace client; merge both sides' new locale keys - Align DTO, Cargo.lock, bun.lock, Cargo.toml and package.json to main; backend Rust code is now identical to main - Fix leftover from the previous merge: the plugin CLI refactor (#167) front/back mismatch that broke the frontend build - Restore dependencies silently downgraded by the previous merge (jsonc-parser, toml_edit, tauri plugins, eslint, knip, tailwindcss) to their main versions - Reconnect new layout's "refresh all marketplaces" updateMarketplace client and mutation against the existing plugins-market/update endpoint
- Fixes clippy::unnecessary_sort_by that failed CI lint (clippy -D warnings)
- global-search: reset active index in the onChange handler - search: sync draft from urlQuery via render-phase adjustment instead of an effect - opencode-panel: drop form-reset effect; dialog mounts on open so useState initializers suffice
# Conflicts: # crates/api/src/routes/inference.rs
Fldicoahkiin
changed the title
Fldicoahkiin
changed the title
Summary by CodeRabbit
New Features
Improvements