Provider plugin system: post-review fixes and runtime improvements#304
Merged
Provider plugin system: post-review fixes and runtime improvements#304
Conversation
- IPluginLogger interface in abstractions package
- PluginFileLogger writes to ~/.polypilot/logs/plugins/{name}/plugin.log
- PluginLoader logs all discovery, hash checks, loading steps
- CopilotService.Providers logs provider registration and init
- ISessionProviderFactory.ConfigureServices gets optional logger param
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- GetServices<ISessionProvider>() was lazy — construction errors during foreach iteration were uncaught and silently swallowed - Now .ToList() materializes eagerly inside try/catch - Full stack trace logged to _system plugin log for diagnosability Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Plugin's ILogger<T> was a different type than host's because Microsoft.Extensions.Logging.Abstractions wasn't in the shared assembly list. Added Logging, Logging.Abstractions, Options, and Hosting.Abstractions to prevent type identity conflicts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Plugins with native dependencies (e.g., e_sqlite3) need the load
context to probe runtimes/{rid}/native/ for .dylib/.so files.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Host uses AI.Abstractions 10.2.0 but plugins may bundle 10.3.0, causing MissingMethodException on SessionConfig.set_Tools due to AIFunction type identity conflict. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ions MAUI apps launched from Finder don't inherit the terminal's PATH, so spawned tools (az, gh, git) can't be found or see auth state. Ensure common tool directories are on PATH and HOME/AZURE_CONFIG_DIR point to the user's profile so az-cli finds its cached tokens. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The initial SyncProviderMembers in RegisterProvider runs before the provider has loaded its agents. Added a second sync after InitializeAsync to pick up the actual squad members. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…, test fixes - CopilotService.Providers.cs: Clear ToolCallCount, IsResumed, and flush content on error in all 4 provider event handlers (OnTurnEnd/OnError) - PluginLoader.cs: Add Path.GetFullPath containment check to prevent path traversal attacks in plugin loading - Tests: Add PluginFileLogger compile include, widen contextmenu proximity threshold for provider attributes on session-item div Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
1 participant
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.
Summary
Follow-up to #292 — addresses PR review feedback and fixes runtime issues discovered during end-to-end plugin testing.
PR Review Fixes
OnTurnEnd/OnErrorfor leader and member sessions) now clearToolCallCount,IsResumed, and flush accumulated content on error paths, matching the 9-field cleanup invariant.PluginLoader.LoadEnabledProvidersnow validates that resolved DLL paths stay within the plugins root directory viaPath.GetFullPath()containment check.PluginFileLogger.cscompile include to the test project.Runtime Improvements (from e2e testing)
IPluginLoggerinterface to abstractions +PluginFileLoggerimplementation. Plugins get a dedicated logger that writes to~/.polypilot/plugins/{name}/plugin.log, giving plugin authors runtime diagnostics without coupling to the host logging system.ISessionProviderFactoryinstances from the service provider with full exception logging, catching assembly load failures at startup rather than at first use.Microsoft.Extensions.Logging,Microsoft.Extensions.Hosting,Microsoft.Extensions.AI, and native library resolution with the plugin load context. PreventsTypeIdentityConflictwhen plugins reference the same framework assemblies as the host.PATH,HOME, andAZURE_CONFIG_DIRto providerCopilotClientOptionsso plugins that spawn CLI processes (e.g., copilot) inherit the correct environment.SyncProviderMembersafterInitializeAsynccompletes to pick up any members the provider created during initialization.Testing
Closes review items from #292.