Skip to content

Allow custom provider endpoint edits#1007

Merged
PeterDaveHello merged 1 commit into
masterfrom
fix/custom-provider-endpoint-edit
Jul 16, 2026
Merged

Allow custom provider endpoint edits#1007
PeterDaveHello merged 1 commit into
masterfrom
fix/custom-provider-endpoint-edit

Conversation

@PeterDaveHello

@PeterDaveHello PeterDaveHello commented Jul 16, 2026

Copy link
Copy Markdown
Member

Stop blocking custom provider endpoint updates just because saved conversations still reference the provider.

Keep delete protection tied to API modes and saved conversations, while removing the now-unused endpoint rewrite guard and its tests.

Summary by CodeRabbit

  • New Features

    • Provider endpoints can now be edited without being blocked by saved conversations.
    • Improved provider matching during session recovery, including legacy configuration handling.
    • Preserves the selected API mode when editing a different provider row.
  • Bug Fixes

    • Simplified provider editing validation to focus on provider name and a valid API URL.
    • Removed outdated saved-conversation warning messages.

@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 842a2d82-d1d1-4578-a9d5-c9816a6a6c5a

📥 Commits

Reviewing files that changed from the base of the PR and between ecc1d44 and 072e756.

📒 Files selected for processing (3)
  • src/popup/sections/ApiModes.jsx
  • src/popup/sections/api-modes-provider-utils.mjs
  • tests/unit/popup/api-modes-provider-utils.test.mjs
💤 Files with no reviewable changes (1)
  • src/popup/sections/api-modes-provider-utils.mjs

📝 Walkthrough

Walkthrough

Provider editing no longer validates saved-conversation dependencies when changing provider endpoints. Session reference resolution is consolidated into label-based provider matching, and tests are updated for referenced IDs and persisted mode selection.

Changes

Provider reference validation

Layer / File(s) Summary
Session reference resolution
src/popup/sections/api-modes-provider-utils.mjs, tests/unit/popup/api-modes-provider-utils.test.mjs
Provider IDs referenced by sessions are resolved through provider IDs, legacy custom URLs, and consolidated label matching; obsolete endpoint-reference APIs and tests are removed.
Provider editor validation
src/popup/sections/ApiModes.jsx
Provider draft saving validates only the name and API URL, and removes saved-conversation validation state, guards, handlers, and UI errors.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

Suggested labels: Review effort 4/5

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: custom provider endpoint edits are now allowed despite saved conversation references.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/custom-provider-endpoint-edit

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request removes the logic and validation that blocks provider endpoint rewrites based on saved conversations. Specifically, it deletes several helper functions such as isProviderEndpointRewriteBlockedBySavedConversations and isProviderReferencedBySessionsViaUrl from api-modes-provider-utils.mjs, cleans up the corresponding state and UI validation in ApiModes.jsx, and removes their associated unit tests. I have no feedback to provide as there are no review comments.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@kilo-code-bot

kilo-code-bot Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (27 files)
  • .github/workflows/scripts/verify-search-engine-configs.mjs
  • .github/workflows/verify-configs.yml
  • badges/coverage.json
  • src/_locales/*/main.json
  • src/background/index.mjs
  • src/components/ConversationCard/index.jsx
  • src/components/InputBox/index.jsx
  • src/components/InputBox/resize.mjs
  • src/config/index.mjs
  • src/config/openai-provider-mappings.mjs
  • src/content-script/styles.scss
  • src/popup/sections/ApiModes.jsx
  • src/services/apis/provider-registry.mjs
  • tests/unit/components/input-box-resize.test.mjs
  • tests/unit/services/apis/provider-registry.test.mjs

Note: Incremental review over previous SHA ecc1d44. The existing comment at src/popup/sections/ApiModes.jsx:303 was authored by qodo-code-review[bot] and concerns design intent (legacy session endpoint resolution), outside this run's changed-line scope; no new Kilo findings were produced.

Previous Review Summary (commit ecc1d44)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit ecc1d44)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (3 files)
  • src/popup/sections/ApiModes.jsx
  • src/popup/sections/api-modes-provider-utils.mjs
  • tests/unit/popup/api-modes-provider-utils.test.mjs

Reviewed by hy3:free · Input: 69.5K · Output: 4.2K · Cached: 430.8K

@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Allow editing custom provider endpoints even when saved conversations exist

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Allow custom provider endpoint edits without blocking due to saved conversation references.
• Keep provider delete protection tied to API mode usage and loaded saved conversations.
• Remove the unused endpoint-rewrite guard logic and its associated unit tests.
Diagram

graph TD
  A["ApiModes UI"] --> B["Provider editor"] --> C["Validate URL"] --> D["Persist provider changes"]
  A --> E["Load saved sessions"] --> F[("Browser storage")]
  A --> G["Reference checks"] --> H["Delete protection"]
  C --> I["provider-utils"]
  D --> I
  E --> I
  G --> I
  subgraph Legend
    direction LR
    _ui["UI"] ~~~ _util["Utility"] ~~~ _store[("Storage")]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Replace hard-block with a warning banner on endpoint edits
  • ➕ Preserves user freedom to edit while still surfacing potential impact on recovery
  • ➕ Keeps guardrail value without preventing legitimate endpoint rotations
  • ➖ Adds UI/translation complexity and requires product decision on severity
  • ➖ Still risks confusing users if warning appears frequently
2. Auto-migrate saved sessions/provider references on endpoint change
  • ➕ Reduces future recovery ambiguity by updating stored references to the edited provider
  • ➕ Could make provider identity more stable across endpoint rotations
  • ➖ Higher risk: mutates user history data and requires careful backward compatibility
  • ➖ Needs robust handling of multiple providers, keys, and legacy session shapes
3. Keep the block only for ambiguous legacy recovery cases
  • ➕ Maintains protection where endpoint change could break recovery for sessions lacking stable IDs
  • ➕ Smaller behavioral shift vs removing the block entirely
  • ➖ Complex to define and test “ambiguous” correctly
  • ➖ May still block common, valid endpoint updates depending on legacy data mix

Recommendation: Proceed with the PR’s approach: editing a provider endpoint should not be blocked by saved-conversation references, since delete protection already covers the risky operation and the removed guard was both complex and now effectively unused. If user confusion becomes an issue, consider adding a non-blocking warning rather than reintroducing a hard failure.

Files changed (3) +50 / -923

Bug fix (1) +3 / -61
ApiModes.jsxRemove saved-conversation endpoint rewrite block from provider edit flow +3/-61

Remove saved-conversation endpoint rewrite block from provider edit flow

• Eliminates the provider endpoint rewrite validation that prevented saving edits when saved conversations referenced the provider. Keeps endpoint format validation and leaves delete protection logic based on provider references and session load state.

src/popup/sections/ApiModes.jsx

Refactor (1) +0 / -166
api-modes-provider-utils.mjsDelete unused saved-conversation endpoint rewrite guard helpers +0/-166

Delete unused saved-conversation endpoint rewrite guard helpers

• Removes the exported endpoint rewrite blocking function and its supporting session-recovery matching helpers that were used to enforce the previous edit restriction. The remaining session reference extraction and delete-disable helpers stay intact.

src/popup/sections/api-modes-provider-utils.mjs

Tests (1) +47 / -696
api-modes-provider-utils.test.mjsRemove unit tests for removed endpoint rewrite guard and URL-only reference checks +47/-696

Remove unit tests for removed endpoint rewrite guard and URL-only reference checks

• Drops the test suite coverage for the deleted endpoint rewrite guard and its URL-based session reference function. Retains and adjusts tests around session recovery and referenced provider ID extraction.

tests/unit/popup/api-modes-provider-utils.test.mjs

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes the “endpoint rewrite” save-guard that previously prevented editing a custom provider’s Chat Completions endpoint when saved conversations could still resolve to that provider. It keeps provider deletion protection tied to “is referenced by API modes or saved conversations” and to the “saved conversations loaded” state, while allowing endpoint edits to proceed.

Changes:

  • Removed the saved-conversation-based endpoint rewrite blocker and its related helper logic from provider utilities.
  • Simplified provider editor validation in the popup UI to validate only provider name + endpoint URL (no saved-conversation guard).
  • Deleted the corresponding unit tests for the removed guard logic and updated remaining tests accordingly.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
tests/unit/popup/api-modes-provider-utils.test.mjs Removes unit tests for the deleted endpoint rewrite guard and updates remaining session-recovery expectations.
src/popup/sections/ApiModes.jsx Removes the UI validation that blocked saving provider endpoint edits due to saved conversations; keeps delete protection logic intact.
src/popup/sections/api-modes-provider-utils.mjs Deletes the now-unused endpoint rewrite guard helpers and related URL-based session reference checker used only for that guard.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@qodo-code-review

qodo-code-review Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Legacy URL recovery breaks ✗ Dismissed 🐞 Bug ≡ Correctness
Description
With the saved-conversation endpoint rewrite guard removed, editing a provider’s Chat Completions
URL can make some legacy saved conversations (missing/stale providerId and relying on customUrl
matching) stop resolving to that provider and continue using their stored session.apiMode.customUrl
instead. This can leave those conversations pinned to an obsolete endpoint and cause requests to go
to the wrong/old URL after a provider endpoint update.
Code

src/popup/sections/ApiModes.jsx[R294-303]

    const endpointDraft = validateProviderEndpointDraft(providerDraft.apiUrl)
    const parsedEndpoint = endpointDraft.parsedEndpoint
-    const providerEndpointChanged =
-      Boolean(providerEditingId) &&
-      Boolean(persistedProvider) &&
-      parsedEndpoint.valid &&
-      parsedEndpoint.chatCompletionsUrl !== resolveProviderChatEndpointUrl(persistedProvider)
-    const effectiveProviderSecrets =
-      pendingDeletedProviderSecretIds.length > 0
-        ? applyDeletedProviderSecrets(config.providerSecrets, pendingDeletedProviderSecretIds)
-        : config.providerSecrets
    const nextProviderDraftValidation = {
      name: !providerName,
      apiUrl: !endpointDraft.valid,
-      savedConversations:
-        providerEndpointChanged &&
-        isProviderEndpointRewriteBlockedBySavedConversations(
-          providerEditingId,
-          sessionsLoaded,
-          sessions,
-          effectiveProviders,
-          configuredCustomApiModesForSaveGuard,
-          effectiveProviderSecrets,
-        ),
    }
-    if (
-      nextProviderDraftValidation.name ||
-      nextProviderDraftValidation.apiUrl ||
-      nextProviderDraftValidation.savedConversations
-    ) {
+    if (nextProviderDraftValidation.name || nextProviderDraftValidation.apiUrl) {
      setProviderDraftValidation(nextProviderDraftValidation)
      if (nextProviderDraftValidation.name) {
        providerNameInputRef.current?.focus()
Evidence
The popup no longer blocks saving provider endpoint changes based on saved conversations, while the
request routing/session recovery code can still resolve legacy sessions via a URL match against the
provider’s current endpoint; changing that endpoint removes the match and may cause fallback
behavior that uses the session’s stored customUrl.

src/popup/sections/ApiModes.jsx[287-308]
src/popup/sections/api-modes-provider-utils.mjs[397-419]
src/services/apis/provider-registry.mjs[463-480]
src/services/apis/provider-registry.mjs[590-607]
src/services/apis/provider-registry.mjs[638-757]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Editing a custom provider endpoint is now allowed even when saved conversations exist; however, some *legacy* sessions resolve providers by matching `session.apiMode.customUrl` against the provider’s configured URL. After a provider URL edit, those sessions may no longer match the provider and can fall back to using the session’s persisted `customUrl`, meaning the provider edit doesn’t apply to those conversations (and may break them if the old URL is no longer valid).

### Issue Context
- `buildEditedProvider` updates `chatCompletionsUrl` when the URL changes.
- Provider/session resolution for legacy sessions matches by `customUrl` first; if no match, `resolveOpenAICompatibleRequest` can fall back to legacy behavior that uses the session’s stored `customUrl`.
- Since the popup no longer blocks saves based on saved conversations, this mismatch can now be triggered by normal endpoint edits.

### Fix Focus Areas
- src/popup/sections/ApiModes.jsx[287-353]
- src/popup/sections/api-modes-provider-utils.mjs[397-419]
- src/services/apis/provider-registry.mjs[463-500]
- src/services/apis/provider-registry.mjs[590-607]
- src/services/apis/provider-registry.mjs[638-757]

### Suggested fix
Implement one of:
1) **Migration on save (preferred for correctness):** when a provider endpoint is changed and `sessionsLoaded` is true, find sessions where:
  - `session.apiMode.groupName === 'customApiModelKeys'`
  - providerId is missing/stale (doesn’t resolve to any enabled provider)
  - `session.apiMode.customUrl` matches the provider’s *previous* endpoint URL
  Then update those sessions to track the provider (e.g., set `apiMode.providerId` to the edited provider id and clear `apiMode.customUrl`) and persist back to storage.

2) **Explicit warning/confirm:** if such sessions exist, show a confirmation/warning that X saved conversations will keep using their old `customUrl` and won’t follow the provider endpoint edit unless migrated.

Either approach keeps the new “allow edits” behavior while avoiding silent legacy-session breakage.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread src/popup/sections/ApiModes.jsx
Stop blocking custom provider endpoint updates just because saved
conversations still reference the provider.

Keep delete protection tied to API modes and saved conversations, while
removing the now-unused endpoint rewrite guard and its tests.
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 072e756

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request removes the 'isProviderEndpointRewriteBlockedBySavedConversations' function and its associated helper methods, along with the corresponding unit tests. This cleanup simplifies the provider endpoint validation logic by removing the dependency on saved conversation checks. I have no further feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@PeterDaveHello
PeterDaveHello merged commit 03e9c9b into master Jul 16, 2026
5 checks passed
@PeterDaveHello
PeterDaveHello deleted the fix/custom-provider-endpoint-edit branch July 16, 2026 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants