Skip to content

Fix/improve#205

Draft
mmilanta wants to merge 2 commits intomainfrom
fix/improve
Draft

Fix/improve#205
mmilanta wants to merge 2 commits intomainfrom
fix/improve

Conversation

@mmilanta
Copy link
Copy Markdown
Contributor

@mmilanta mmilanta commented Mar 6, 2026

No description provided.

Copy link
Copy Markdown

@JiwaniZakir JiwaniZakir left a comment

Choose a reason for hiding this comment

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

The deletion of MCPScanner.py is a substantial change, but the truncated diff makes it impossible to verify whether the functionality is being migrated elsewhere or simply removed — the PR title "Fix/improve" provides no clarity on this.

Looking at the code being deleted: get_servers_from_path has a control-flow bug where the UnknownMCPConfig branch sets result.error but then falls through to servers = mcp_config.get_servers() without an early return or else guard, meaning it would attempt to enumerate servers from an unrecognized config anyway. The synchronous __exit__ using asyncio.run(self.context_manager.wait()) would also raise a RuntimeError if called from within an already-running event loop, which is a realistic scenario when this scanner is embedded in async applications. Additionally, ContextManager.emit() appends coroutines to self.running but never clears that list after wait() completes, so a reused ContextManager instance would re-await already-completed coroutines on subsequent wait() calls. If these issues are the motivation for the rewrite, they should be called out explicitly in the PR description with references to what replaces this file.

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