Skip to content

feat(ai): add OpenAIProvider behind the registry - #384

Draft
dobrinyonkov wants to merge 6 commits into
split/pr1-provider-refactorfrom
split/pr2a-openai-provider
Draft

feat(ai): add OpenAIProvider behind the registry#384
dobrinyonkov wants to merge 6 commits into
split/pr1-provider-refactorfrom
split/pr2a-openai-provider

Conversation

@dobrinyonkov

Copy link
Copy Markdown
Contributor

Stacked on #381 (PR1). Base is split/pr1-provider-refactor; it will auto-retarget to master when PR1 merges.

What this does

Adds a second provider — OpenAIProvider — behind the Provider interface and registry that PR1 introduced. This is a foundation slice: the provider is registered and fully unit-tested, but Gemini Nano stays the default and there is no UI to switch to OpenAI yet. The settings modal and storage-driven provider selection follow in a later PR.

Four commits:

  1. Add OpenAIProvider behind the registry. fetch + SSE streaming, Bearer auth, [DONE] sentinel, cross-read line buffering, cancellation via AbortSignal, and API-error surfacing that never leaks the apiKey. checkAvailability returns ready only when baseUrl, apiKey, and model are all set (no network ping). Registered under 'openai' with a configSchema.
  2. Move network I/O into the service worker. The panel cannot reach arbitrary origins under the extension CSP, so the provider proxies fetch/stream through a background/openaiHandler.js port protocol.
  3. CSP fix so the service worker fetch is allowed.
  4. Stop hardcoding the Gemini ready-message in the controller. AssistantController had three 'Gemini Nano is ready' strings that clobbered the banner on setUrl, clearConversation, and post-streaming recovery. Cache the active provider's own ready message and re-emit that instead. Invisible until a non-Gemini provider is actually selected.

No user-visible change

AIChat.js is byte-identical to PR1 — providerName stays 'gemini-nano', providerConfig stays {}. An end user sees exactly the PR1 behavior. OpenAI is reachable only by constructing the controller with providerName: 'openai' and a config, which nothing in the shipped UI does yet.

What is deliberately NOT here

  • No settings modal, gear icon, or provider picker.
  • No storage-driven selection or setProvider hot-swap.
  • No not-configured banner action or token-counter/feature-detection UI work.

All of that is the next slice.

Testing

npx grunt karma:CI — 614 tests pass. eslint and jshint clean.

Manual (pending a live gateway): construct AIChat with providerName: 'openai' + gateway config, confirm checkAvailability reports ready and a send round-trips through the service worker (fetch appears under the extension's service worker in the Network tab, not the panel). Confirm the default Gemini path is untouched.

Review path

Read OpenAIProvider.js and background/openaiHandler.js (the port protocol between them). providers/index.js is the one-line registry entry. The AssistantController.js change is the ready-message cache. Everything else is tests.

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. Gemini Nano stays the
  default; OpenAI is registered but not user-selectable until the
  settings UI lands in a later slice.
- 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.

(cherry picked from commit 0e79d64537cf8073ea6f64ea66aef8f4cf4db5c6)
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.

(cherry picked from commit 830f4ad01d547e6394e80046309cc3259135c34c)
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'.

(cherry picked from commit 2b8673871ac4cc32ecd047a930f7d8ef8a2db696)
Collapse the identical 401/404/429 error tests into one table-driven
loop, and export parseSseEvent so the SSE edge cases become direct unit
tests instead of full fetch+stream+port harness setups. Adds coverage
for keep-alive lines, non-JSON payloads, and empty choices.
OpenAIProvider was missing getUsageInfo, which AssistantController calls
unconditionally after every stream-complete. With OpenAI selected the
first reply threw a synchronous TypeError that escaped the panel's
promise chain. Add a stub returning null; the token pill already handles
null by leaving its last state in place.

destroy() also cleared _disconnectHandler without invoking it, so a
clear/setUrl during an active stream left the sendMessage promise
permanently pending, leaking the port and abort listener. Reject the
pending promise before tearing down the port.
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.

1 participant