Skip to content

fix(v2): refresh connected providers after late host registration - #373

Merged
EyJunge1 merged 2 commits into
tickernelz:mainfrom
doublepi123:fix/v2-provider-refresh
Oct 5, 2026
Merged

EyJunge1 merged 2 commits into
tickernelz:mainfrom
doublepi123:fix/v2-provider-refresh

Conversation

@doublepi123

Copy link
Copy Markdown
Contributor

Summary

On OpenCode v2, auto-capture and profile learning can report a configured provider as "not connected" for the lifetime of the server process.

The plugin reads the provider directory once during setup. At that point OpenCode has only registered its built-in opencode provider; user-configured providers appear about two seconds later, together with provider.updated and model.updated events. The snapshot therefore stays at ["opencode"], so a provider configured via opencodeProvider (for example a custom OpenAI-compatible provider) fails the connection gate before any generation request is made.

This was reproduced with a probe plugin against an isolated OpenCode 2.0.21 server: ctx.provider.list() returned only opencode during setup and opencode plus the configured provider two seconds later. HTTP /api/provider showed the configured provider as enabled while the plugin kept reporting it as disconnected.

Changes

  • Refresh the connected-provider snapshot on provider.updated and model.updated. Bursts are coalesced into at most one trailing refresh; refreshes stop after dispose.
  • Before failing the connectivity gate in auto-capture and profile learning, refresh once and re-check (ensureProviderConnected). The error message is unchanged.
  • Refresh errors are logged and keep the previous snapshot. A disposing plugin instance only clears its own refresher.
  • The activation filter and provider-directory semantics from fix: harden provider discovery, capture scheduling and learning locks #369 are unchanged.

Test plan

  • New tests/v2-provider-refresh.test.ts uses the real plugin, V2 adapter event path, provider module state and auto-capture gate. Covers late registration, refresh-on-miss, unknown providers, refresh errors, event bursts and dispose.
  • The new tests fail on the current main and pass with this change.
  • Existing loader mocks gained ensureProviderConnected.

Checklist

  • Branch is based on current main (a24d79e, v2.28.3).
  • bun test passes (672 pass / 0 fail / 4 platform skips).
  • bun run typecheck passes.
  • bun run check passes.
  • Docs: no configuration or contribution-process change.

OpenCode v2 registers user-config providers a few seconds after plugin
setup. At setup, ctx.provider.list() only returns the built-in `opencode`
provider; configured providers appear later together with
provider.updated/model.updated events. The plugin read the directory once
during init, so a configured provider such as `newapi` stayed "not
connected" for the lifetime of the process and every capture/profile
request failed before generation.

- Refresh the connected-provider snapshot on provider.updated and
  model.updated, coalescing bursts into at most one trailing refresh and
  stopping after dispose.
- Before failing the connectivity gate, refresh once and re-check
  (ensureProviderConnected). The error message is unchanged.
- Refresh errors are logged and keep the previous snapshot. A disposed
  instance only clears its own refresher.
@doublepi123
doublepi123 force-pushed the fix/v2-provider-refresh branch from 718ad2f to c576c1b Compare October 5, 2026 04:31

@EyJunge1 EyJunge1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review summary

Approve — the miss-refresh path (ensureProviderConnected) correctly fixes the late provider-registration bug for auto-capture and profile learning. CI is green and the regression coverage for the gate path looks solid.

Not fully optimal: the event-driven refresh is defense-in-depth but lightly guaranteed, and a couple of comments/wiring details don't match the code. None of that blocks the bugfix.

What works well

  • Refresh-on-miss before failing the connectivity gate (auto-capture + profile LLM)
  • Snapshot retained on refresh errors; existing error strings unchanged
  • Dispose clears the refresher by function identity (multi-instance safe)
  • Activation / provider.list() semantics from #369 left alone
  • Subprocess fixture covers miss, ghost, capture-error, refresh-error, dispose

Nits (non-blocking follow-ups)

  1. Event path vs eventBelongsToLocation
    V2 adapter drops events with neither location.directory nor a resolvable sessionID (src/v2/legacy-client.ts). provider.updated / model.updated are ephemeral with optional location. If the host publishes without location, the event handler never runs and only miss-refresh saves you. Worth confirming real host payloads include location, and giving burst-fixture events a directory so the coalescing test actually exercises the adapter path.

  2. Comment vs code — gate bypasses coalescing
    Comment says ensureProviderConnected reuses runCoalescedRefresh, but providerRefresher calls refreshConnectedProviders() directly → parallel list() possible under event + miss. Either wire through coalescing or fix the comment.

  3. Init refresh outside the mutex
    void refreshConnectedProviders() doesn't set refreshInFlight. Prefer runCoalescedRefresh() for the initial call so early events don't race a second list.

  4. Minor

    • Log still says "Failed to initialize…" on later refreshes
    • Burst assertion listCalls < 8 is soft; a tighter coalesce bound (or location-bearing events) would be stronger
    • Optional unit tests for ensureProviderConnected / clear-by-identity

Verdict

Merge as-is is fine. The gate miss-refresh is the load-bearing fix; the nits above are polish / hardening for a follow-up if you want them.

Comment thread src/index.ts Outdated
})();
};

void refreshConnectedProviders();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit: This init call bypasses the coalesce mutex below (refreshInFlight is still unset). Prefer runCoalescedRefresh() so a concurrent provider.updated during init cannot start a second parallel provider.list().

Share one awaitable coalesced refresh for init, events and gate misses; forward location-less provider/model inventory events; tighten burst coverage.
@EyJunge1
EyJunge1 merged commit 8168422 into tickernelz:main Oct 5, 2026
7 checks passed
@EyJunge1 EyJunge1 mentioned this pull request Oct 5, 2026
3 tasks
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