Repository navigation
fix(extensions): diagnose installed connector connections - #291
Conversation
External review follow-up and merge readinessThank you to the external reviewer for the code-level checks and precise evidence corrections, and to Desolate-Jix for the connector revisions and clear lifecycle boundaries. The maintainer-relayed external review accepts the five technical review points at Retained non-blocking work
The shared transport also serves Audit wording corrections
Evidence and repository ownershipHost connection code and the live verification record remain in this repository. The unpublished October 29 static/byte review is now available as a sanitized record on Extensions #11, with reviewer responsibility, date, curated-v1 policy, exact artifact digest and evidence scope. All 2,803 raw live-evidence files have been copied into a private archive outside the disposable worktree and individually verified after reopening the ZIP. Original failed attempts, the initial audit and the static-review source are retained. Raw profiles and logs were not uploaded. The next Host gate is an eligible non-author GitHub approval, followed by the normal protected merge. This comment records the supplied external review; it is not an 简体中文感谢外审逐项核对代码、纠正证据措辞，也感谢 Desolate-Jix 持续返修连接器并明确生命周期边界。外审接受 TLS 证书错误误归类已单独留档为 #292，补充了不联网的合成异常链复现，按低优先级后续处理。共享展示函数、将来 OAuth 的 401 质询顺序、旧的超时中文提示也保留为非阻塞后续事项。共享传输的另一个调用方 审计三处更正：extension-settings smoke 既有本地也有 CI 通过证据；Pi 新入口提示的实机覆盖仅限缺服务，错误 token 最终文案为回归覆盖；实机运行于 ffb1037 加当时未提交的改动，随后提交为 8a09ae3，不能补造当时不存在的提交或 diff 摘要。 0.0.8 的独立静态/字节审查已公开在上面链接的 Extensions #11 评论。2,803 份原始实机文件已在 worktree 外私有归档并逐文件校验，失败记录保留，未上传原始 profile 或日志。Host 代码及实机文档继续留在 Host 库；Extensions 只保存收录、批准与发布事实并引用 Host permalink。 接下来需取得有权限的非作者 GitHub Approve，再走正常合并。当前评论用于记录维护者转来的外审结论，不冒充 GitHub 正式批准；Host 合并后再补 #11 的精确版本收录记录并完成后续批准和独立索引发布。 |
Merge receiptThe maintainer explicitly authorized a one-time administrator bypass after accepting the external review. The exact reviewed head No non-author GitHub The permanent merged Host live-verification record will be referenced by the 0.0.8 catalog input in Extensions #11. Thank you again to the contributor and external reviewer. 简体中文维护者接受外审结论后，明确授权本次管理员 bypass。已再次确认精确 head 本次没有非作者 GitHub APPROVED 事件，该门槛由维护者明确豁免；未修改仓库保护配置。TLS 后续项保留在 #292，其他发现与措辞补正在上方留档。Extensions #11 将引用上述已合入 Host 的固定实机记录链接。再次感谢贡献者与外审。 |
## What and why The MCP connection probe added in #291 classifies every `httpx2.ConnectError` as `connection_unreachable`. httpcore2 maps an `ssl.SSLError` raised during the TLS handshake (including certificate verification failures) to `ConnectError`. The mapped exception survives in the exception chain, including `__context__` after the connection pool re-raises with `from None`, so an HTTPS server with a self-signed, expired or mismatched certificate was reported as "Could not reach the MCP server" and the certificate reason was lost in both the plugin card and Settings → Providers → MCP (details in #292). `_probe_error` now unwraps only the httpx2/httpcore2 `ConnectError` layers and checks whether the mapped failure is an `ssl.SSLError`. Genuine certificate/protocol errors fall through to the existing `connection_failed` result with the redacted underlying diagnostic. Other connection failures, including a refused connection or a peer closing during the TLS handshake, keep the actionable unreachable message. In particular, the classifier does not look through anyio's `EndOfStream`/`BrokenResourceError` to an incidental SSL error retained in their context. TLS validation is unchanged. Fixes #292 Linked Issue for product-semantic or public-contract changes: none (routine fix) ## Change class - [x] Routine fix, documentation, test, maintenance, or presentation-only UI - [ ] Product-semantic or public-contract change discussed in the linked Issue - [ ] Isolated, default-off experiment Owning layer: `server/handlers/mcp_connection_handler.py`, the Host handler that classifies `mcp.connection.test` probe failures for both the plugin card and Settings → Providers → MCP. User-visible effect, or `none`: a TLS failure while testing an HTTPS MCP connection shows its certificate reason (for example `CERTIFICATE_VERIFY_FAILED ...`) again, instead of the "check that it is running and that the URL and port are correct" message. Compatibility or migration impact, or `none`: none. TLS failures return the `connection_failed` code they returned before #291; other codes are unchanged. ## Evidence Commands and manual journeys run: - `python -m pytest -q tests/test_mcp_connections.py`: 17 passed (macOS, CPython 3.12, `mcp==2.2.0`, `httpx2`/`httpcore2` 2.12.0). - The new test `test_http_probe_keeps_tls_certificate_diagnostics_without_credentials` runs the handler through the real MCP client with a transport that raises a `ConnectError` caused by `ssl.SSLCertVerificationError`. It fails on `main` (`connection_unreachable`) and passes with this change; it also checks the bearer token is redacted from the detail. - The existing unreachable case now raises its `ConnectError` from a `ConnectionRefusedError`, so it checks that a non-TLS `OSError` cause still gives `connection_unreachable`. - Manual check without mocks: tested the handler against a local HTTPS server with a self-signed certificate and against a closed port. On `main` both returned `connection_unreachable`. With this change the HTTPS server returns `connection_failed` with the certificate-not-trusted reason, and the closed port still returns `connection_unreachable`. - `ruff check server/handlers/mcp_connection_handler.py tests/test_mcp_connections.py` and `git diff --check`: passed. - [x] Relevant Python tests pass - [x] CPU/model-less baseline remains supported - [ ] Electron `npm run build` passes when Electron code changed - [ ] Dependency audit passes when dependencies changed - [ ] Before/after screenshots are attached for visible UI changes - [ ] Documentation/examples are updated for changed settings or contracts Maintainer follow-up validation (`0a8d7046`, Windows, CPython 3.12.10, MCP 2.2.0, httpx2/httpcore2 2.12.0, anyio 4.14.2): - Added certificate-free loopback cases using the real HTTP/TLS stack: an HTTP response to ClientHello preserves the SSL diagnostic; a peer that reads ClientHello and closes retains `connection_unreachable`. The tests bypass ambient proxies, bound fixture lifetime, and await accepted connections during cleanup. - Before/after: the protocol-error case fails on the main baseline, and the peer-close case fails on the original PR head. Both pass after narrowing the classifier. - `PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 python -m pytest -p pytest_asyncio.plugin -q tests/test_mcp_connections.py`: 19 passed. Existing certificate-diagnostic and credential-redaction coverage remains in place. - `python -m ruff check server/handlers/mcp_connection_handler.py tests/test_mcp_connections.py` and `git diff --check`: passed. ## Final check - [x] This PR addresses one coherent problem without unrelated cleanup - [x] It does not add a speculative API, fallback, or compatibility path - [x] No secrets, local state, model weights, voice material, or restricted assets are included - [x] Third-party notices and provenance are preserved --------- Co-authored-by: Lucas1479 <sli776@aucklanduni.ac.nz>
Installed MCP connectors lacked a connection-test action on their plugin card, and HTTP authentication failures could collapse into generic protocol errors. The card now probes the actual Host-issued connection, shows actionable 401/403/unreachable diagnostics without credentials, and hides obsolete probes while settings await restart. Native Pi warnings point to the correct settings, and a cancelled refresh cannot leave a stale reconnecting warning after restart.
The added verification record closes the independent connector 0.0.8 + Runtime 0.1.11 four-step GUI, original-image review, observe/continue, operator-window preservation and official settle checks for Amadeus-Extensions #11. It distinguishes test-harness failures, actual results and remaining catalog gates. No package approval or index is published.
Validation on cd4ab09: all 15 remote checks passed. The complete local Electron suite passed 706 tests with zero skips; the two initially failing fixture/locale modules passed all 32 tests after fixing the fixture inputs and key ordering, without relaxing assertions. Six existing Work completion tests also passed. The unchanged production implementation retains the earlier 45 targeted Python passes, Electron build and 14 production-React smoke checks. Real UI and installer evidence is linked in the verification record. This PR remains a draft for independent external review; no merge or catalog approval has been performed.
Compatibility:
mcp.connection.listadds an optionalextension_package_idonly for package-owned connections; existing IDs and manual-connection output remain unchanged. Error codes refine the existing probe failure response. Native non-authentication MCP negotiation remains covered.中文：本 PR 修复已安装连接器缺少测试入口、身份验证错误提示不明确及重启后残留警告的问题，并保存 0.0.8 + 0.1.11 的独立实机验收记录。四步 GUI、原图审核、observe/continue、窗口保留、官方 settle 与安装器生命周期已通过；这不等于插件已批准上架。最新提交的 15 项远端检查已全部成功，本地完整 Electron 测试 706 项通过。初次 CI 的夹具缺参和翻译键排序问题已修复，断言未放宽。按维护者要求保持草稿，等待独立外审；尚未合并或补入 #11 收录记录。