feat: add pi coding agent extension package with MCP adapter integration#42
feat: add pi coding agent extension package with MCP adapter integration#42pikujs wants to merge 3 commits into
Conversation
Add a self-contained pi coding agent extension package to the existing computer-use-linux repository so users can install it with `pi install npm:@agent-sh/computer-use-linux` and gain Linux desktop control through pi-mcp-adapter's MCP proxy tool. Key changes: - Create pi/extension/index.ts: TypeScript extension that auto-discovers the computer-use-linux binary (env var → npm bundled → PATH) and writes the MCP server config to ~/.pi/agent/mcp.json for pi-mcp-adapter to consume. - Refactor skills/computer-use-linux/SKILL.md: Make the skill generic and host-agnostic. Move Hermes-specific install/config steps to references/hermes-setup.md and add a new Pi setup guide at references/pi-setup.md. The core Procedure, When to Use, and Pitfalls sections remain unchanged — they were already MCP-generic. - Update package.json: Add `pi` manifest key pointing to the extension and skill, `pi-package` keyword, and include the new pi/ and references/ directories in the published package. No npm dependency on pi-mcp-adapter — it remains a documented prerequisite to avoid pulling pi-only packages for regular npm install users. - Update README.md: Add Pi Coding Agent install section after the existing Hermes Agent section.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for the Pi coding agent, including a new TypeScript extension to auto-register the computer-use-linux binary as an MCP server, updated package configuration, and restructured documentation with host-specific setup guides for Hermes and Pi. The review feedback highlights two important robustness improvements in the new extension: validating the parsed JSON structure in readMcpConfig to prevent runtime crashes or configuration corruption, and wrapping synchronous filesystem operations in a try-catch block to handle potential write errors gracefully.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| function readMcpConfig(path: string): McpConfig { | ||
| try { | ||
| const raw = readFileSync(path, "utf-8"); | ||
| return JSON.parse(raw) as McpConfig; | ||
| } catch { | ||
| return {}; | ||
| } | ||
| } |
There was a problem hiding this comment.
If the mcp.json config file is empty, corrupted, or contains a non-object value (such as null or an array), JSON.parse can return a value that is not a plain object. This leads to two issues:
- If it returns
null, accessingconfig.mcpServerswill throw aTypeErrorat runtime, crashing the extension. - If it returns an array, spreading it with
{ ...config }will convert it into an object with numeric keys (e.g.,{ "0": ..., "mcpServers": ... }), corrupting the configuration file.
We should validate that the parsed JSON is a non-null, non-array object before returning it.
function readMcpConfig(path: string): McpConfig {
try {
const raw = readFileSync(path, "utf-8");
const parsed = JSON.parse(raw);
if (parsed && typeof parsed === "object" && !Array.isArray(parsed)) {
return parsed as McpConfig;
}
return {};
} catch {
return {};
}
}| const configPath = getPiAgentMcpConfigPath(); | ||
| const changed = ensureServerEntry(configPath, binaryPath); | ||
|
|
||
| if (changed && ctx.hasUI) { | ||
| ctx.ui.notify?.( | ||
| `${PACKAGE_NAME}: MCP server configured at ${configPath}. ` + | ||
| "Run /reload if pi-mcp-adapter is already installed.", | ||
| "info", | ||
| ); | ||
| } |
There was a problem hiding this comment.
The call to ensureServerEntry performs synchronous filesystem operations (readFileSync, mkdirSync, writeFileSync). If any of these operations fail (e.g., due to permission issues, a read-only filesystem, or disk full), an unhandled exception will be thrown inside the async session_start event handler. This can lead to unhandled promise rejections or crash the Pi agent process.
We should wrap the configuration update in a try-catch block to handle filesystem errors gracefully and notify the user of the failure.
const configPath = getPiAgentMcpConfigPath();
try {
const changed = ensureServerEntry(configPath, binaryPath);
if (changed && ctx.hasUI) {
ctx.ui.notify?.(
PACKAGE_NAME + ": MCP server configured at " + configPath + ". " +
"Run /reload if pi-mcp-adapter is already installed.",
"info",
);
}
} catch (err) {
ctx.ui.notify?.(
PACKAGE_NAME + ": Failed to update MCP config at " + configPath + ": " + (err instanceof Error ? err.message : String(err)),
"warning",
);
}- Add isRecord() type guard to validate JSON.parse result in readMcpConfig() before casting to McpConfig - Wrap mkdirSync/writeFileSync in try-catch in writeMcpConfig() to handle permission errors gracefully - Log write failures to console instead of crashing
| args: ["mcp"], | ||
| }; | ||
|
|
||
| writeMcpConfig(configPath, { ...config, mcpServers: servers }); |
There was a problem hiding this comment.
This is an auto review done by revuto.
writeMcpConfig now catches mkdirSync/writeFileSync failures and only logs them, but ensureServerEntry still returns true unconditionally after this call. That makes the session handler notify that the MCP server was configured even when mcp.json was not written (permission denied, read-only FS, disk full, etc.), leaving pi-mcp-adapter with no computer-use-linux entry and no visible desktop tools. Please propagate the write failure (or have writeMcpConfig return a success boolean) so the extension can warn the user instead of reporting a successful configuration.
|
Hey @pikujs, thanks for this. I re-checked the current head and there are two things I think we should fix before merging:
Once those are fixed this should be in much better shape. |
Error handling hardening (per review audit):
- Add accessSync(constants.X_OK) to PATH search so non-executable
files on PATH are skipped instead of failing at spawn time
- Log a warning when mcp.json contains valid JSON that is not a
plain object (e.g. array), instead of silently returning null
- Wrap session_start async handler in try-catch to prevent
unhandled promise rejections from unexpected errors
- Return null from readMcpConfig when file exists but can't be
read/parsed, so ensureServerEntry knows not to overwrite it
Doc fixes:
- Fix pi-setup.md and README.md examples to use correct
server-prefixed tool names (computer_use_linux_doctor)
- Add mcp({ server: ... }) listing pattern to both docs
|
I checked your reviews and hardenned the exception handling |
|
|
||
| // Write/update the MCP config that pi-mcp-adapter reads | ||
| const configPath = getPiAgentMcpConfigPath(); | ||
| const changed = ensureServerEntry(configPath, binaryPath); |
There was a problem hiding this comment.
This is an auto review done by revuto.
ensureServerEntry now returns false for both “already up to date” and “could not read/write the config” (see the config === null and write-error paths above). Because the session handler only notifies when changed is true, a corrupt/unreadable mcp.json or a failed write leaves pi-mcp-adapter without the computer-use-linux server entry but shows no UI warning as long as the mcp tool itself is installed. Please distinguish the no-op and failure cases (or surface a warning from the failure path) so users get actionable feedback when registration did not actually happen.
2 participants
Summary
Add a self-contained pi coding agent extension package to the existing
computer-use-linuxrepository. Users can now runpi install npm:@agent-sh/computer-use-linuxand get Linux desktop control tools throughpi-mcp-adapter's MCP proxy — without maintaining a separate pi-specific skill or duplicating tool definitions.Key Changes
New: Pi extension (
pi/extension/index.ts)computer-use-linuxbinary on first load (checksCOMPUTER_USE_LINUX_BINenv var → npm-bundled binary →$PATH)session_start, writes/merges the MCP server entry into~/.pi/agent/mcp.json— the config file thatpi-mcp-adapterreadspi-mcp-adapteris missing, notifies the user with actionable instructionsRefactored: Skill (
skills/computer-use-linux/SKILL.md)references/hermes-setup.mdreferences/pi-setup.mdUpdated: Package metadata (
package.json)pimanifest pointing to the extension and skillpi-packagekeyword for pi package registry discoverypi/andreferences/to the npm tarball files listUpdated: README
Design Decisions
pi-mcp-adapter's proxy tool.npm installusers (Hermes, Claude, etc.) shouldn't pull pi-only packages. It's documented as a prerequisite.~/.pi/agent/mcp.json— the same filepi-mcp-adapterreads. Merge strategy: reads existing config, updates only thecomputer-use-linuxentry.Verification
cargo installon PATH)~/.pi/agent/mcp.jsoncomputer_use_linux_doctor,computer_use_linux_list_windows,computer_use_linux_clickverified end-to-endnpm pack --dry-runincludes all new filesNotes
.gitignorechange (.agent-shell/and.agent-docs) is a pre-existing local modification, unrelated to this featuretoolkit-accessibility = true(runnable viagsettingsor Hyprlandexec-once)Fixes: N/A — feature addition