Conversation
|
👋 This PR needs a couple of things fixed before OpenHands can review it:
Push an update once this is addressed and this check re-runs automatically. This is an automated check - no AI was used to generate this comment. |
|
The latest PR Description Check now passes. The required |
|
🚦 CI is currently failing on this PR's latest commit. Please fix the failing checks before OpenHands reviews it - this is re-checked automatically once you push a new commit. (A maintainer can also request This is an automated check - no AI was used to generate this comment. |
all-hands-bot
left a comment
all-hands-bot
left a comment
There was a problem hiding this comment.
This review was posted by an AI agent (OpenHands).
Summary
The change adds one hand-authored integrations/catalog/pilot-protocol.json plus the regenerated integrations/catalog-index.js, which is exactly the layout AGENTS.md and integrations/README.md require. I verified the generated index is correct: running npm run build:integrations on the head produced no diff, so the extra entry77 import/entry pair and renumbering match the build script. uv run --group test pytest tests/test_catalog_schema.py tests/test_integration_catalog_in_sync.py tests/test_catalogs.py -q -> 115 passed, and uv run --group test python scripts/sync_extensions.py --check passes (one pre-existing, non-blocking coverage warning for plugins/issue-duplicate-checker). Only the pr-title and Validate PR description checks are reported for this head; the required test, sync-extensions, and validate-claude-code workflows are absent from the check list, so the CI evidence in the PR description could not be corroborated from the API.
Material finding: the entry cannot connect as configured
I probed the published package the entry actually launches (npx -y pilotprotocol-mcp, pilotprotocol-mcp@0.2.13). The stdio server starts and lists 21 tools, including pilot_search and pilot_peers as claimed, but every tool call fails:
initialize -> serverInfo pilot-mcp 0.2.13
tools/list -> 21 tools (pilot_search, pilot_peers, ...)
tools/call pilot_peers -> isError: true, "pilotctl binary not found. Run `pilot-mcp setup` or install pilot-daemon first."
tools/call pilot_search -> isError: true, "pilotctl binary not found. Run `pilot-mcp setup` or install pilot-daemon first."
The backend is a separate Go pilotctl binary that the bare stdio invocation does not provision. The package has no optionalDependencies, and the platform subpackages its own resolver looks for (pilot-mcp-<platform>) do not exist on npm (404). Provisioning happens only through npx -y pilotprotocol-mcp setup, which fetches a release archive, extracts daemon/pilotctl into ~/.pilot/bin, and starts a background daemon. The catalog installs the bare command, so a marketplace install yields a server whose every tool returns an error, with nothing in the catalog telling the user what to do about it.
This matters to the merge decision because the catalog contract explicitly requires entries to be actionable and to avoid making clients guess security- or UX-relevant defaults (integrations/README.md: "These requirements keep catalogs actionable and prevent clients from guessing security- or UX-relevant defaults"). The repository already has precedent for documenting a required local prerequisite through installHint: quickbooks describes its one-time OAuth handshake, sonarqube describes its required token/container runtime, and git describes its uvx path. This entry has neither installHint nor notes, and its single option declares auth.strategy: "none", which presents the integration as zero-setup in the install modal. The PR's own "How to Test" only confirms the server starts and exposes the two tools, which is the part that works; it does not cover a tool call, which is the part that does not.
Secondary, non-blocking
connectionOptions[0].id is "local", an id that appears nowhere else in the catalog; every other single-option no-auth stdio entry uses "none" (time, fetch, memory, playwright, sequential-thinking, everything, browser-mcp). The type is "oauth" | "api" | "none" | string, so this is schema-valid, but unless a consumer is known to branch on "local", "none" keeps the entry consistent with the rest of the catalog.
The advertised behavior is broader than "discover specialist agents and exchange messages with trusted peers": installing the runtime places a persistent background daemon on the user's machine and opens P2P UDP networking, and the package documents a managed mode in which pre/post action envelopes, including tool inputs and results, are sent to a hosted control plane for policy evaluation. That mode is opt-in per the package's own README, but it is the kind of system-level and data-flow behavior the installHint/notes fields exist to surface.
Suggested resolution
Add an installHint (and/or notes) recording that the integration requires a one-time npx -y pilotprotocol-mcp setup to provision the pilotctl daemon before any tool works, and summarize the daemon/network side effects. No other change is needed; the catalog file, its id, and the regenerated index are all correct.
🔄 CHANGES REQUESTED
| ] | ||
| }, | ||
| "auth": { | ||
| "strategy": "none" |
There was a problem hiding this comment.
auth.strategy: "none" presents this integration as requiring no setup, but the command in transport.args (npx -y pilotprotocol-mcp) only starts the MCP shim. Every tool call fails until the separate Go pilotctl runtime is provisioned via npx -y pilotprotocol-mcp setup, which downloads a release archive, extracts daemon/pilotctl into ~/.pilot/bin, and starts a background daemon.
I verified this against pilotprotocol-mcp@0.2.13: tools/list returns 21 tools including pilot_search and pilot_peers, but tools/call on both returns isError: true with pilotctl binary not found. Run pilot-mcp setup or install pilot-daemon first. The package declares no optionalDependencies, and the pilot-mcp-<platform> subpackages its resolver prefers are not published (npm returns 404).
Please add an installHint (and optionally notes) stating the one-time setup step and the daemon/network side effects, following the existing precedent for local prerequisites (quickbooks, sonarqube, git). Without it, a marketplace install produces a dead server with no in-catalog explanation.
This branch has not been deployed
3 participants
Why
OpenHands users can run Pilot Protocol as a local stdio MCP server without credentials, but it is not available in the integration catalog.
Summary
pilotprotocol-mcppackage over stdioIssue Number
Related catalog infrastructure: #241
Submission request: #537
How to Test
uv run --group test pytest tests/ -q.pilot_searchandpilot_peers.Automated result: 801 passed, 24 skipped.
Video/Screenshots
Not applicable for the catalog data change.
Notes
Disclosure: I contribute to Pilot Protocol. AI-assisted; reviewed and validated against the repository schema and test suite.
HUMAN: No manual UI test was performed. The repository test suite, both synchronization checks, generated integration index, schema validation, and Claude plugin validation were run locally; the exact results are recorded above.