feat(ai): multi-provider support (Slices 1–3)#361
Open
dobrinyonkov wants to merge 22 commits into
Open
Conversation
dobrinyonkov
force-pushed
the
multi-provider-support
branch
from
d3b6c86 to
aecb7a6
Compare
kgogov
approved these changes
Jul 7, 2026
kgogov
previously approved these changes
Jul 7, 2026
… interface
Introduces the Provider abstraction (three required methods: checkAvailability,
sendMessage, destroy; two optional: downloadModel, getUsageInfo) and swaps
PromptClient out for GeminiNanoProvider — a single file that speaks the
background port protocol internally.
- PromptBuilder gains buildMessages({appInfo, history, userMessage,
inspectionContext, consoleErrors}) returning the full [system, ...history,
wrappedUser] array on every send. buildSeedMessages is removed.
- GeminiNanoProvider caches the messages the underlying Prompt API session was
seeded with. sendMessage compares messages.slice(0, -1) to the cache; if they
differ (URL change, clearConversation, port disconnect from an idle-killed
service worker), it recreates the session before streaming.
- AssistantController takes a provider instead of a promptClient. Loses the
session-lifecycle vocabulary: no _pendingReseed, no _trackReseed, no explicit
createSession calls, no hasActiveSession checks. sendUserMessage builds the
messages array once and hands it to provider.sendMessage(messages, {onChunk}).
- PromptClient.js and PromptClient.spec.js are deleted. Coverage moves to
GeminiNanoProvider.spec.js (same fake-port pattern, plus prefix-cache and
cancellation scenarios).
- AssistantController.spec.js is rewritten around a fake Provider.
First slice of the multi-provider-support work. Registry and OpenAIProvider
land in follow-up slices.
Refs: .scratch/multi-provider-support/issues/01
AssistantController now constructs its provider through a static registry
instead of taking one directly. The registry lives at modules/ai/providers/
index.js and holds a single entry so far — 'gemini-nano' — with the shape
{displayName, ProviderClass, configSchema} the settings modal will render
against in a later slice.
- providers/index.js exports the PROVIDERS map and a createProvider(name,
config) factory. Unknown names throw.
- AssistantController takes {providerName, providerConfig, createProvider}
instead of {provider}. createProvider defaults to the registry factory and
doubles as a test seam so specs can swap in a fake without registering it.
- AIChat passes 'gemini-nano' and an empty config through to the controller.
- AssistantController.spec.js uses the createProvider seam to inject its
fake Provider. No new registry-standalone test — it's a trivial static
object, covered transitively by the controller specs.
No user-visible change. Validated end-to-end against the mock harness
(port init → send → clear) with all existing behaviours intact.
Refs: .scratch/multi-provider-support/issues/02
Implements Slice 3 of multi-provider-support. - OpenAIProvider: fetch + SSE streaming, Bearer auth, [DONE] sentinel, cross-read line buffering, cancellation via AbortSignal, API-error surfacing without leaking apiKey. checkAvailability returns 'ready' only when baseUrl, apiKey, and model are all set (no network ping). - Registered under 'openai' with displayName 'OpenAI-compatible' and a configSchema for baseUrl / apiKey / model. - Temporarily wires AIChat to the openai provider with the test gateway config so the end-to-end path can be validated before the settings UI lands in Slice 4. Both the hardcoded providerName swap and the inlined config are marked for removal in Slice 4. - Spec covers SSE split-chunk buffering, [DONE] sentinel, 401/404/429 error surfacing, apiKey redaction, cancellation, and checkAvailability config-presence logic. Fake fetch injected at the constructor seam. - Add AbortSignal to jshintrc globals (Chrome-supported alongside the already-present AbortController).
Implements Slice 3.5 of multi-provider-support.
The panel's CSP (default-src 'self') blocks cross-origin fetch, so all
network I/O for the OpenAI-compatible provider now runs in the background
service worker. Panel-side OpenAIProvider becomes a thin port-protocol
client, symmetric to GeminiNanoProvider.
- OpenAIProvider (panel): connects lazily to a chrome.runtime port named
'openai-api', posts {type:send, config, messages}, routes chunk /
complete / error frames. Cancellation posts {type:cancel}. destroy()
posts cancel and disconnects. No fetch or SSE parsing in the panel.
- openaiHandler (background): new module, one AbortController per port.
On send, fetches ${baseUrl}/chat/completions, parses the SSE stream,
posts chunk / complete / error frames back. Redacts config.apiKey out
of any error message it echoes back over the wire. Aborts on cancel
or port disconnect.
- main.js dispatches port.name === 'openai-api' to attachOpenAIHandler
alongside the existing 'prompt-api' branch.
- OpenAIProvider.spec.js rewritten in the fake-port style used by
GeminiNanoProvider.spec.js: request-message shape, chunk/complete/error
handling, cancellation posts {type:cancel}, disconnect surfaces an
error, destroy disconnects.
- New openaiHandler.spec.js covers SSE split-chunk buffering, [DONE]
sentinel, 401/404/429 error surfacing, apiKey redaction (asserted as
a full-message substring check), fetch rejection, cancel aborts the
in-flight fetch, and port disconnect aborts the fetch.
Manifest CSP and host_permissions were already in the clean state on
this branch — no revert needed.
Slice 3.5 moved OpenAIProvider's HTTP I/O into the background service
worker on the theory that host_permissions alone would cover the
cross-origin fetch. That theory is wrong on current Chrome — the
extension_pages CSP applies to the service worker too, and its
default-src 'self' falls back for connect-src, blocking any endpoint.
Add an explicit connect-src using CSP scheme-source expressions:
connect-src 'self' http: https:
The scheme-source form ("http:", "https:") is what Chrome's CSP
parser accepts for allow-any-host-over-scheme. Host-source with a bare
wildcard ("http://*") parses without error but does not match any
host — a subtle CSP gotcha that cost us a debugging round.
No new attack surface: the extension already declares access to every
http(s) origin via host_permissions. Slice 4's settings UI can tighten
this to user-specified origins if that becomes worthwhile.
- AssistantController.setProvider(name, config): aborts in-flight stream via AbortController, destroys old provider, constructs new one via registry, re-checks availability. Conversation memory kept. - sendUserMessage now propagates an AbortSignal to the provider. AbortError from the swap does not emit stream-failed nor flip capability to streaming-failed. - AIChat gains providerName/providerConfig options; hardcoded gateway config removed. - Panel wiring reads ai_provider_name and ai_provider_config from chrome.storage.local; falls back to gemini-nano with empty config. - AssistantController.spec.js: setProvider tests covering old-provider destroy, factory args, history preservation, in-flight abort, capability re-emit, and quiet-abort behaviour.
Adds AISettingsModal — a proper modal (backdrop, focus trap, Esc to close) opened from a gear icon in the AI Chat header. The modal reads each provider's configSchema from the registry to render its form, pre-fills from the stored per-provider config, and validates that all required fields are non-empty before Save is enabled. Save persists ai_provider_name and ai_provider_config to chrome.storage.local (merging into any existing per-provider config so credentials for other providers survive) and calls controller.setProvider(name, config). AIChat gains three constructor options (providersRegistry, storage, settingsModalFactory) so the modal path is fully injectable for tests and the view keeps its production defaults.
- AssistantController.getUsageInfo now resolves to null when the current provider does not implement the optional method, per the PRD. Without this, switching from Gemini to OpenAI at runtime crashed the view's post-save token-counter refresh with 'getUsageInfo is not a function'. - Group the download, clear-history, and settings buttons in a .banner-actions wrapper. The banner's justify-content: space-between was distributing the buttons individually, leaving a large gap between Clear History and the gear icon. The wrapper keeps them clustered.
Adds an optional `placeholder` field to configSchema entries and threads it through to the rendered input. OpenAI-compatible now hints 'https://host/v1', 'your API key', and 'e.g. gpt-4o-mini' — generic enough not to leak any real endpoint or credential.
AssistantController had three hard-coded 'Gemini Nano is ready' strings
that clobbered the banner whenever setUrl, clearConversation, or a
post-streaming-failure recovery ran. With OpenAI configured, opening
DevTools showed the correct provider banner momentarily, then setUrl
fired and the banner reverted to 'Gemini Nano is ready'.
Cache the last known ready message from the provider's own
checkAvailability, and re-emit that on every subsequent ready
transition. Also give OpenAIProvider a self-identifying ready message
('OpenAI-compatible (<model>) ready') so the banner names the active
provider instead of a generic 'Ready'.
…tion Slice 6 of the multi-provider refactor. - AssistantController.getProviderCapabilities() reports whether the currently-installed provider implements the optional downloadModel and getUsageInfo methods. AIChat consults it inside _renderCapabilityBanner and _updateTokenCounter, so switching from Gemini to OpenAI hides the token counter and download button on the next capability-state emission. - OpenAIProvider.checkAvailability tags the missing-config unavailable result with reason: 'not-configured' and a user-facing 'Open settings to configure' message. The controller plumbs reason through _setCapabilityState so the view can distinguish it from browser-level unavailable reasons. - AIChat gains a banner action button (id ai-banner-action) that is only shown when reason === 'not-configured'. Clicking it opens the settings modal via the same _openSettings path as the gear icon. - Extended _CAPABILITY_CONFIG so 'unavailable' and 'unsupported' now carry disableInput: true. _onCapabilityStateChanged applies the flag via _applyInputDisabledState, and re-enables the input on 'ready'. Manually validated with the mock harness: OpenAI mode hides the token counter and download button; the not-configured banner shows the Open settings action; clicking it opens the modal; the input is disabled on unavailable/unsupported states.
…ing credentials Two bugs surfaced during manual validation in the actual extension: 1. Token counter stayed visible when swapping from Gemini (active conversation) to OpenAI while OpenAI resolved to unavailable (missing config). _updateTokenCounter was only invoked on the 'ready' state via the updateTokens config flag, so a swap that landed on unavailable never re-ran feature detection and the pill kept its Gemini value. Call _updateTokenCounter on every mapped capability transition. The hide-when-hasUsageInfo-false early return already handles the hide; the null-usage no-op still prevents mid-stream flicker on the ready path. 2. Settings modal disabled Save whenever any required field was empty. That prevented users from clearing credentials — e.g. removing an API key while keeping OpenAI selected. Drop the required-field validation entirely. Save is always enabled; the provider's checkAvailability surfaces missing config as an unavailable/not-configured capability state, which already drives the banner, banner action, and input-disabled UX.
Bump per-section character caps in PromptBuilder from 800/400 to 8000 (properties, bindings, aggregations, console errors) and BINDING_VALUE_CAP from 100 to 1200 (~300 tokens). Update tests that reference the old cap sizes.
Collapse three state-machine test clusters into table-driven form and drop the redundant real-settings-modal end-to-end test. - Empty-conversation gating: introduce runScenario + rebuild helpers, fold four terminal-state scenarios into one table-driven it(), merge the two attribute checks into one, merge the hide-on-clear and reveal-after-clear tests into one cycle test. 10 tests -> 5. - Assistant Capability State routing: iterate class-name mapping and message-text surfacing internally in a single it() (excluding the downloading percent-format special case), iterate download-button visibility rule internally in a single it(). 12 tests -> 7 (unmapped, class-name, downloading class-name, download-button visibility, downloading progress-percent, session-failed clear-history, and streaming-failed skip). - Inline error slot: one parametrised keydown-clears test over ['a', 'Control', 'Enter', 'ArrowLeft'] replacing the two dedicated keydown tests. Also drops the redundant 'resumes typing' input-event case, whose input-event _clearError coverage remains in the 'sends a new message' test. - Settings modal: delete 'should drive the real settings modal end-to-end'; real-modal behaviour is covered in AISettingsModal.spec; wiring is covered by the six fake-modal tests that remain. Add a one-line orientation comment above the describe block. Coverage preserved: every previous assertion has a home. karma:CI green (638 total; 66 it() in AIChat.spec, was 77). Note: the two numeric acceptance criteria (test count -15, LOC -250) undershot at -11 tests and -114 LOC. Meeting them would have required sacrificing coverage or dropping in-scope constraints from the safety pre-check (e.g. re-folding 'downloading' into the class-name test). Coverage was prioritised per pre-check bullets 2 and 3.
…ngsModal.spec - Merge password/text input-type tests into one parametrised table, adding a coverage-expansion row for undefined type (defaults to 'text'). - Merge with-placeholder and no-placeholder tests into one parametrised table; keep hasAttribute() as the assertion axis so the 'no placeholder' row can't silently pass on any input. - Delete 'should render zero form fields when the selected provider has an empty configSchema' — trivially covered by [].forEach being a no-op. Test count in the file drops from 24 to 21 (−3). LOC drops from 350 to 329 (−21), which is below the ≥40 target in the issue; the arithmetic of the parametrisation (5 tests removed × ~7 LOC, 2 parametrised tests added × ~16 LOC) does not permit −40 without expanding scope beyond the four target tests. All 638 karma:CI tests pass.
- Extract _setClearHistoryVisible(bool): folds four call sites that
looked up #ai-clear-history-button and toggled style.display
('conversation-loaded', 'conversation-cleared',
_onCapabilityStateChanged, _handleSendMessage). The
_onCapabilityStateChanged gate '(status !== ready || _hasMessages)'
stays at the call site so per-status logic is unchanged.
- Extract _resetTokenCounterClasses(counter): keeps the two internal
call sites in _updateTokenCounter but removes the duplicated class
list. Folding into one unconditional reset was considered and
rejected — the null-usageInfo path intentionally leaves prior
classes on the pill to avoid flicker (see the existing 'Null usage'
comment). The AC line 'exactly one place' conflicts with task 2's
explicit permission for either shape; helper-extract is the
behavior-preserving choice.
- Inline _checkModelAvailability(): the one-line wrapper is deleted
and init() calls _controller.initialize() directly.
- Trim five JSDoc blocks (_render, _CAPABILITY_CONFIG, _hideContextPill,
_showError, _clearError) to one-liners or delete where the method is
obvious. Non-obvious invariants (transcript ownership of the messages
container, unknown-status fallback, show-path asymmetry for the
context pill, auto-clear behavior of the error slot) are preserved
in the one-liners.
Net LOC saved: 24 (not the 60 the AC targeted; user-accepted).
All 638 unit tests remain green.
Two passes over the multi-provider-support work.
Pass 1 — simplifications (zero behavior change):
- GeminiNanoProvider: capability-state lookup table, .every() for
messagesEqual, shared cleanup helper in downloadModel/_createSession
- openaiHandler: dedup fallback in extractErrorMessage; err?.message
form kept as (err && err.message) || … (browserify 16 / acorn does
not parse optional chaining)
- AssistantController: _msg(err, fallback) helper (4 sites), ternary
getUsageInfo, single push with two args
- PromptBuilder: .filter(Boolean) for section accumulation, ternary
helpers, filter().forEach() for history
- AIChat: fold _clearError into _showError(''), loop settings/banner
click bindings, inline _resetTokenCounterClasses,
updateContext(null) reuses _hideContextPill()
- AISettingsModal: drop dead dataset.required, data-driven click
bindings
Pass 2 — dead code, each finding adversarially verified with two
independent Explore skeptics (2×14 = 28 refutation attempts). Only
findings both skeptics failed to refute were deleted; four claims
survived refutation and were kept, including a real race in the
debounced flush and a session-leak guard on the download path.
- AssistantController._isStreaming: 5 writes, 0 production readers
(AIChat has its own separate flag)
- _CAPABILITY_CONFIG['session-failed']: no provider emits it; drop
the entry, the LESS rule, and the associated guard branch
- background/main.js: session-destroyed port message had no listener
- AssistantTranscript.scrollToBottom: dead scrollHeight === undefined
half of the guard
- AssistantTranscript.appendUserTurn/appendSystemMessage: discarded
HTMLElement return values
- AssistantController.sendUserMessage: { content } resolution never
read in production (stream-complete event delivers the same object)
- consoleErrorCapture.uninstall: never invoked in production
- AssistantTranscript._appendMessage: showCopyButton === true branch
unreachable (no caller passes true)
Tests: 637 passing (dropped one obsolete session-failed spec).
JSHint: clean.
Net: 13 files, +93 −195 lines.
dobrinyonkov
force-pushed
the
multi-provider-support
branch
from
a3fea95 to
90f6f65
Compare
Widen the Prompt Builder's property section to carry the selected control's
inherited property groups and a typed, cleaner property line for each entry.
- controlUtils: additive typeNames sibling on properties.own and each
properties.inheritedN, carrying the raw declared type string. Preserve
meta.controlName on inherited groups so the prompt heading can name the
declaring class. types dictionary and DataView consumers are unchanged.
- PromptBuilder: replace the flat {value, isDefault} property line shape
with '- <name>: <TypeName> = <value>', appending '(default)' when the
entry is marked default and omitting the type slot when typeNames[key]
is empty. Strings are JSON-quoted (so embedded quotes stay escaped);
booleans, numbers, null, undefined render literally; objects and arrays
are JSON.stringify'd and capped at 500 characters. Combined 8000-char
budget across own + all inherited groups with outer-first truncation:
deepest inherited group is dropped whole; own alone overflow is
truncated with '... [truncated]'.
Closes issue 01 of .scratch/ai-prompt-control-api-reference.
…ed properties Widen the Prompt Builder's control block with an 'Enums used:' subsection that lists the full set of valid values for each enum-typed property on the selected control. The subsection lands after all property groups and before Bindings:. - Detection per property: typeof types[key] === 'object' AND typeNames[key] is a non-empty string. Members come from Object.keys(types[key]) in insertion order (which matches the UI5 enum's declared order at runtime). - Line format: '- <enum type name>: V1 | V2 | V3 | ...'. One line per unique enum type across own + all inherited groups; dedup key is the type name. - Section capped at 2000 characters via a new module-level ENUMS_CAP constant, consistent with the existing per-section caps. Truncation appends the standard '... [truncated]' marker. - Custom-library enums (object types[key], missing typeNames[key]) are silently skipped from the subsection; the corresponding property line is unaffected. Non-enum properties (types[key] is a primitive string) never produce an entry. - Subsection omitted entirely when no property has a resolvable enum. No change to controlUtils, AssistantController, or any other consumer. Closes issue 02 of .scratch/ai-prompt-control-api-reference.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements Slices 1–3 of the multi-provider-support PRD (
.scratch/multi-provider-support/PRD.md).Slices in this PR
Slice 1 —
fdaa94arefactor(ai): rename PromptClient to GeminiNanoProvider with Provider interfacePromptClient→GeminiNanoProviderwith the new three-methodProviderinterface (checkAvailability,sendMessage,destroy) plus optionaldownloadModel/getUsageInfo.AssistantControllerloses all Gemini-specific vocabulary (sessions, reseeds,_pendingReseed,hasActiveSession).messagesarray ({role, content}[]) is now the single canonical format across storage, controller, prompt builder, and provider — no transform layer.Slice 2 —
27812d2refactor(ai): add provider registry with Gemini registeredapp/scripts/modules/ai/providers/index.js: static registry mapping provider name →{displayName, ProviderClass, configSchema}.AssistantControllerconstructs its provider viacreateProvider(name, config)at the constructor seam.Slice 3 —
d3b6c86feat(ai): add OpenAIProvider with hardcoded gateway configOpenAIProvider:fetch+ SSE streaming, Bearer auth,[DONE]sentinel, cross-read line buffering, cancellation viaAbortSignal, API-error surfacing without leakingapiKey.checkAvailabilityreturnsreadyonly whenbaseUrl,apiKey, andmodelare all set (no network ping).'openai'withdisplayName'OpenAI-compatible'and aconfigSchemaforbaseUrl/apiKey(password) /model.AIChatis hardcoded to theopenaiprovider with the test-gateway config so the end-to-end path can be validated before the settings UI lands. Both the hardcoded name-swap and the inlined config are marked for removal in Slice 4.Testing
grunt karma:CI).tests/modules/ai/GeminiNanoProvider.spec.js— replacesPromptClient.spec.js; covers the port protocol at the constructor seam.tests/modules/ai/OpenAIProvider.spec.js— 22 tests covering SSE split-chunk buffering,[DONE], packed multi-event reads, role-only openers, 401/404/429/500 error paths,apiKeyredaction, cancellation, andcheckAvailabilityconfig-presence.AssistantController.spec.jsupdated to use the newProviderfake at the interface boundary.Not in this PR (later slices)
ai_provider_name/ai_provider_configfromchrome.storage.local, removes the hardcoded provider swap fromAIChat.AISettingsModaland the gear icon in the AI Chat header.getUsageInfo/downloadModel).Verification via mock harness
Slice 3 was validated end-to-end using the OpenAI mode of the local mock harness driven through chrome-devtools:
checkAvailability→readywith baseUrl/apiKey/model set.${baseUrl}/chat/completionswith correct body (model=gpt-5.4, stream=true) andAuthorization: Bearer …header (only last 4 chars logged).streaming-failedwith the API's error message;apiKeynever appeared in any log line.