Versioned provider plugin API with selected-only entry-point loading (#171)

[hellices]

Closes #171

What changed

Public provider API v1

• New supported boundary: korvid.agent.provider_plugin
  • PROVIDER_PLUGIN_API_VERSION = 1
  • frozen ProviderPluginMetadata / ProviderPluginConfig
  • ProviderPlugin ABC
  • ValidatedPluginProvider
• Plugins receive only neutral config + CredentialSource; no Kubernetes,
  UI, tool executor, approval, proposal, or audit objects.
• Event validation allows only text_delta, tool_call, usage, done;
  strict types and bounds (64 KiB text/arguments, 256-char IDs/names,
  non-negative usage <= 1e9), extra fields removed, malformed events fail
  before runtime history/tool dispatch is mutated.
• Cleanup is idempotent and delegates to the provider exactly once.

Selected-only entry-point loading

• New ProviderPluginRegistry discovers korvid.provider metadata but calls
  .load() only for the explicitly selected normalized name.
• Real packaged fixture (.dist-info + entry_points.txt) proves unrelated
  entry points are never imported.
• Built-ins/aliases are centralized and reserved; normalized variants
  (openai_compat, github_copilot, whitespace/case variants) stay on the
  built-in path and are rejected directly by the plugin loader.
• Duplicate normalized names report both distributions deterministically.
• Metadata object/field/API/name/auth validation is runtime-enforced and
  bounded; import/metadata/factory errors expose no plugin payload or
  credential text.
• Validated metadata/plugin are cached for one wiring lifetime; metadata is
  read once.

Bounded plugin options

• agent.options supports data-only JSON-like config:
  • depth 4, 64 mapping keys, 64 list items
  • 2 KiB/string, 16 KiB serialized budget
  • finite floats only, string keys only
  • secret-bearing key segments rejected (secret, password, token,
    api_key, authorization, credential, including Unicode lookalikes)
• Invalid options disable only the plugin and surface an actionable startup
  warning; the base TUI remains usable.
• AgentSettings.options is deeply immutable (nested mapping proxies,
  lists frozen to tuples) and preserved across reconnect/model flows without
  being rewritten by the generic wizard.

Wiring and lifecycle

• One registry instance is shared by initial creation, configurator probe,
  and rebuild.
• Initial plugin failures become TUI-visible startup warnings; rebuild
  failures use the existing setup error surface.
• Credential cleanup on construction failure has a strong task reference and
  consumes close errors safely.
• Provider rebuild is transactional: a failed rebuild does not close or
  replace the active provider.
• Rebuild, :ai off, and shutdown close the active plugin provider exactly
  once.
• Base/no-agent startup does not call importlib.metadata.entry_points or
  import plugin modules.

Documentation

• New docs/provider-plugins.md: exact API, complete package/adapter example,
  event/options limits, lifecycle, compatibility, selected-only behavior, and
  trusted-code security warning.
• Agent guide and README distinguish built-ins, OpenAI-compatible gateways,
  and third-party protocol/auth adapters.

Verification

• Task-level TDD and two-stage reviews for all six implementation tasks.
• Final high-capability whole-branch review: approved after fixing metadata
  boundary and reserved-name normalization findings.
• Fresh full gate:
  • ruff: passed
  • mypy strict: passed (285 source files)
  • pytest: 3295 passed, 21 skipped
  • tach: passed

[Copilot]

🟡 Not ready to approve

Provider rebuild atomicity, error containment, option validation, and credential lifecycle have unresolved correctness and security issues.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

Introduces a versioned third-party LLM provider API with selected-only loading, bounded configuration, lifecycle wiring, and documentation.

Changes:

• Adds provider-plugin contracts, event validation, and entry-point discovery.
• Threads immutable plugin options through configuration, UI, and rebuild flows.
• Adds packaged fixtures, extensive tests, and author/operator documentation.

File summaries

File	Description
src/korvid/agent/provider_plugin.py	Defines plugin API and event validation.
src/korvid/agent/provider.py	Updates provider-boundary documentation.
src/korvid/agent/setup.py	Adds immutable plugin options.
src/korvid/providers/plugin_registry.py	Implements selected plugin discovery.
src/korvid/providers/registry.py	Routes custom providers through plugins.
src/korvid/providers/configurator.py	Passes registry and options during probes.
src/korvid/core/config.py	Parses and bounds plugin options.
src/korvid/__main__.py	Wires registry, warnings, rebuild, and cleanup.
src/korvid/ui/app.py	Seeds UI agent options.
src/korvid/ui/widgets/agent_setup_screen.py	Preserves options during setup.
tests/agent/test_provider_plugin.py	Tests contracts and event validation.
tests/agent/test_plugin_runtime.py	Tests runtime integration and isolation.
tests/core/test_config.py	Tests option limits and persistence.
tests/providers/test_plugin_registry.py	Tests discovery, validation, and caching.
tests/providers/test_registry.py	Tests provider factory integration.
tests/providers/test_configurator.py	Tests configurator forwarding.
tests/test_main_wiring.py	Tests lifecycle and transactional wiring.
tests/test_optional_extras.py	Tests lazy discovery and imports.
tests/ui/test_agent_wiring.py	Tests option preservation and errors.
tests/ui/test_agent_setup_screen.py	Tests option immutability and reconnects.
tests/fixtures/provider_plugin/company_provider.py	Provides a valid fixture plugin.
tests/fixtures/provider_plugin/unselected_provider.py	Detects unintended imports.
tests/fixtures/provider_plugin/site_helpers.py	Builds fixture distributions.
tests/fixtures/provider_plugin/__init__.py	Declares the fixture package.
tests/fixtures/__init__.py	Declares shared fixtures.
docs/provider-plugins.md	Documents the public plugin API.
docs/agent.md	Explains plugin selection and trust.
README.md	Links the provider-plugin guide.

Review details

• Files reviewed: 26/28 changed files
• Comments generated: 9
• Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟡 Not ready to approve

Functional normalization gaps and credential/error-boundary leaks remain unresolved.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (3)

src/korvid/providers/registry.py:175

• Plugin authentication failures are only logged and converted to None, so _create_initial_provider cannot add the promised TUI-visible startup warning and rebuilds surface only a generic incomplete-configuration error. Raise a payload-free ProviderPluginError here so startup and rebuild use their existing plugin error surfaces; keep the built-in provider behavior unchanged.

    except _AuthMisconfigured as exc:
        logger.warning("%s — agent disabled", exc)
        return None

src/korvid/providers/registry.py:63

• Normalized Copilot variants do not work in the full startup path. This factory maps github_copilot/case variants to github-copilot, but load_config defaults device login only for the exact spelling (core/config.py:163) and _build_agent_wiring loads the stored OAuth token only for that exact spelling (__main__.py:533). Such a configured variant is therefore routed here with the wrong auth and/or no token and the agent remains disabled. Canonicalize the provider before all provider-specific checks, not only inside this factory.

    name = normalize_provider_name(provider) if isinstance(provider, str) else ""

tests/ui/test_agent_wiring.py:556

• This new asynchronous notification test reintroduces a scheduler-timing wait: one bare pause need not let the rebuild worker finish on slow CI. Poll the observable notification state with until, as required by tests/ui/waits.py:1-4,13-36.

        await pilot.pause()

• Files reviewed: 26/28 changed files
• Comments generated: 3
• Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[hellices]

Round-2 suppressed findings also fixed in 7ad4446: (1) third-party _AuthMisconfigured now becomes a payload-free ProviderPluginError, while built-in log+None behavior is unchanged; startup/rebuild error-surface tests added. (2) provider names are canonicalized during config load before auth/OAuth checks, with normalization parity and full github_copilot startup tests. (3) the async notification test now uses until() condition polling. Independent focused verification: 283 passed; local read-only review approved.

[Copilot]

🟡 Not ready to approve

Stream termination and metadata error handling do not yet fully enforce the documented safety contract.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (3)

src/korvid/agent/provider_plugin.py:91

• The wrapper validates each event shape but never enforces stream termination. If a plugin yields a tool_call and then ends without done (or emits more events after done), AgentRuntime treats exhaustion as success and can dispatch that call. Require exactly one terminal done before allowing the stream to complete.

        try:
            async for event in self._provider.complete(messages, tools, stream=stream):
                yield _normalize_event(event)

docs/provider-plugins.md:106

• This block is labeled the exact API-v1 surface, but complete is documented as a regular method even though the implementation requires an async generator. A plugin author copying this signature would return a synchronous iterator that fails under async for.

    def complete(

src/korvid/agent/provider_plugin.py:185

• The advertised 64 KiB limits are checked with len(str), which counts characters; non-ASCII text or arguments can therefore reach roughly 256 KiB in UTF-8. Keep ID/name limits character-based, but enforce UTF-8 byte length for text_delta.text and tool_call.arguments, with matching docs and Unicode boundary tests.

    value = _require_str(event, label, key)
    if len(value) > max_length:
        raise ProviderPluginContractError(f"{label} exceeds max length {max_length}")

• Files reviewed: 26/28 changed files
• Comments generated: 1
• Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[hellices]

Round-3 suppressed findings fixed in aa30cc1: (1) ValidatedPluginProvider.complete() now requires exactly one terminal done; missing done or any event after done raises before dispatch/history mutation. (2) the exact API example now uses async def complete with yield, matching the required async-generator contract. (3) text and tool arguments are bounded at 65,536 UTF-8 bytes with exact Unicode boundary tests; ID/name remain 256 characters. Independent focused verification: 90 passed; local read-only review approved.

[Copilot]

🟡 Not ready to approve

Malformed oversized API-version metadata can escape the bounded plugin-error path and crash startup.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

• Files reviewed: 26/28 changed files
• Comments generated: 1
• Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟡 Not ready to approve

Common CamelCase credential option keys bypass the new secret-bearing-key rejection.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (1)

src/korvid/providers/plugin_registry.py:41

• These reserved names are not actually shared with built-in dispatch: providers/registry.py:30-40 independently defines _OPENAI_COMPAT_ALIASES and separately checks Ollama/Copilot. A future alias update can therefore route a built-in name into plugin loading or leave it unreserved. Derive both dispatch and collision rejection from one canonical definition.

# Centralized reserved names — these are built-in provider identifiers that must
# never be claimed by third-party plugins.  Shared with korvid.providers.registry
# to ensure plugin_registry and the built-in dispatch cannot drift.
RESERVED_PROVIDER_NAMES: frozenset[str] = frozenset(

• Files reviewed: 26/28 changed files
• Comments generated: 1
• Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[hellices]

Round-5 suppressed provider-name drift finding fixed in 2c33108: OpenAI-compatible aliases plus ollama/github-copilot are defined once in plugin_registry.py; built-in dispatch imports those exact objects and RESERVED_PROVIDER_NAMES derives from them. Identity/structural parity and normalized-variant tests prevent future drift. Full gate reported 3355 passed.

[Copilot]

🟡 Not ready to approve

Authentication defaulting can bypass metadata validation, and the apikey option spelling bypasses secret-key rejection.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (1)

src/korvid/providers/registry.py:177

• build_credentials resolves a missing method to api_key (when api_key_env is set) or none, but ProviderPluginConfig still receives the original None. Because ProviderPluginRegistry.create validates only truthy methods, an api_key-only plugin can be constructed with no credentials, or receive API-key credentials while being told its auth method is None. Resolve the effective method once and pass that same value to credential construction and plugin config so metadata validation cannot be bypassed.

        credentials = build_credentials(name, auth_method, api_key_env)
    except _AuthMisconfigured:
        from korvid.providers.plugin_registry import ProviderPluginError as _PPE

        raise _PPE(f"provider plugin {name!r}: auth misconfigured") from None

• Files reviewed: 26/28 changed files
• Comments generated: 1
• Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[hellices]

Round-6 suppressed auth finding fixed in f7f6f47: the third-party path resolves one concrete effective auth method (auth_method or api_key/none default), passes that exact value to both credential construction and ProviderPluginConfig, and therefore always enforces plugin metadata auth support. Built-in paths are unchanged. Full gate reported 3360 passed.

[Copilot]

🟡 Not ready to approve

Plugin-raised contract errors can leak raw payloads, and option-key validation has pathological allocation behavior.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (2)

src/korvid/agent/provider_plugin.py:92

• A plugin can raise ProviderPluginContractError itself from complete(), and this branch rethrows its raw message. AgentRuntime exposes that message via AgentError, so a plugin exception containing credential text or an unbounded payload bypasses the sanitization applied to every other stream exception. Separate errors raised while advancing the plugin iterator from the wrapper's own validation errors, and sanitize the former regardless of exception class.

        except ProviderPluginContractError:
            raise

src/korvid/core/config.py:618

• Building every contiguous token subsequence materializes O(n²) strings (and O(n³) copied characters) before the 16 KiB serialized-budget check. With several allowed 2 KiB keys containing many separators, config loading can consume hundreds of MB just to test six fixed reserved segments. Compare each fixed segment's token sequence directly instead of constructing the complete subsequence set.

    tokens: set[str] = {normalized}
    for i in range(len(parts)):
        for j in range(i + 1, len(parts) + 1):
            tokens.add("_".join(parts[i:j]))

• Files reviewed: 26/28 changed files
• Comments generated: 0 new
• Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[hellices]

Round-7 suppressed findings fixed in 91a3e37 + e8aeb00: all underlying plugin iterator exceptions (including plugin-raised ProviderPluginContractError) are sanitized, wrapper-generated contract errors remain useful, secret-key detection is now sliding-window O(parts × fixed patterns), and underlying iterators are deterministically aclose()d exactly once on normal/error/cancellation exits without masking primary failures. Independent tests: 189 passed; local re-review approved; full gate reported 3370 passed.

[Copilot]

🟡 Not ready to approve

Plugin-controlled exceptions can still bypass the bounded error boundary during iterator creation and event normalization.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (2)

src/korvid/agent/provider_plugin.py:100

• event is only required to be a Mapping, so calls made by _normalize_event (get, iteration, or field access) can themselves raise. Those exceptions occur outside the translated iterator-advance block and are re-raised raw by the outer handler; AgentRuntime then surfaces their potentially secret or unbounded message. Translate exceptions raised while reading plugin-controlled mappings while preserving Korvid's own fixed validation diagnostics.

                normalized = _normalize_event(event)

src/korvid/agent/provider_plugin.py:84

• The provider's complete() call occurs before the exception boundary. An invalid plugin can satisfy the ABC with a synchronous override that raises here; its raw exception then reaches AgentRuntime, which exposes str(exc) as an AgentError, bypassing the bounded/no-secret plugin error surface. Protect the iterator creation itself as well as iteration.

        iterator = self._provider.complete(messages, tools, stream=stream)

• Files reviewed: 26/28 changed files
• Comments generated: 0 new
• Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[hellices]

Review-loop limit reached: rounds 7 and 8 contained only suppressed advisory findings and no unresolved blocking threads. Credible boundary findings were still fixed in 91a3e37/e8aeb00/b331321: iterator exceptions and hostile Mapping reads are sanitized, secret-key matching is O(n), and iterators close deterministically on every exit. Final local boundary review approved; focused verification 52 passed; agent full gate reported 3375 passed. Per AGENTS.md §8–9, no further Copilot review is requested; proceeding to fresh full-gate and required-check verification.