fix(install): one MCP registration list for all three entry points - #1197
Merged
Conversation
egc init, install.sh and install.ps1 each carried a hand-written copy of the registration list, and the copies had drifted apart: - Neither shell installer ever registered Continue.dev or Zed. Installing through the documented shell command wired up fewer tools than running egc init on the same machine, and nobody would notice because both paths print success. - install.ps1 pointed OpenCode at %APPDATA%\opencode while everything else used the XDG-style path. - Only the shells gated Gemini CLI on Antigravity being absent. The shells now call scripts/lib/mcp-register-cli.js, so there is exactly one list and one set of format handlers (JSON, TOML, Continue YAML, Zed context servers, Claude CLI). Their hand-rolled JSON and TOML writers are gone. The Windows AppData location for OpenCode survives as its own existence-gated target, so no Windows user loses a registration that used to work. Proven in a clean profile with Continue.dev and Zed present: the shell installer now registers both, which it never did before. Suites: mcp-register 34/34, install-sh 6/6, install-ps1 7/7, onboarding 11/11. Signed-off-by: Felipe Marzochi <fmarzochi@gmail.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 6 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Review caught a real regression I introduced by delegating to the shared registry: the shells detected tools with 'command -v', the registry only checked for a config directory, so anyone who had installed Cursor, Kiro, OpenCode or Codex without launching it yet silently stopped getting registered. The registry now accepts either signal. Also: Claude Code was being registered twice on the install.sh path, since the shared list already covers it through the CLI; both installers' leftover Claude blocks are gone. The AppData fallback is computed once instead of four times, so the path a target advertises and the path its gate checks cannot drift. New tests stub platform and APPDATA to exercise the Windows-only target on any CI runner, and cover the on-PATH-but-unconfigured case. The macOS failure was the onboarding assertion comparing a logical temp path against the installer's physical one (/var vs /private/var); it compares realpaths now. Signed-off-by: Felipe Marzochi <fmarzochi@gmail.com>
…lesystem The project path reaches this CLI as a command-line argument and went straight into a file read and write. It is now resolved and checked first: only an existing file literally named .mcp.json is ever opened, so a mistyped or hostile argument cannot make the installer write somewhere else. Security rating on new code was the failing quality gate condition. Signed-off-by: Felipe Marzochi <fmarzochi@gmail.com>
… an argument Passing the path in kept the security gate red: a filesystem path reaching a read and a write from the command line is exactly the pattern the analyzer flags, and validating it after the fact does not remove the source. The filename is now fixed in code and the directory is the process's own working directory, which the installers set to wherever the person invoked them. Nothing about the behavior changes and there is no external path left to validate. Proven in a clean profile: a project .mcp.json in the invoking directory is still picked up and merged. Signed-off-by: Felipe Marzochi <fmarzochi@gmail.com>
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 6 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…ATH probing Four review findings, all real: - On Windows, Git sets HOME to the MSYS home while the installer works out of USERPROFILE, so delegating with HOME first would have registered every tool into the wrong profile. USERPROFILE wins on Windows now. - The package-root exclusion compared raw strings, so reaching the package through a junction or a differently-cased path read as somebody's project and would rewrite the bundled config. It compares canonical physical paths, case-insensitively on Windows. - PATH detection was a second implementation living next to the repo's own. It reuses commandExists from scripts/lib/utils.js, which additionally gained a spawn-free PATH scan fallback: on an image with no 'which' at all, a probe that cannot run looked exactly like 'not installed' and silently skipped a tool the person really had. - install.ps1 merged an existing project .mcp.json twice, once in its own block and once through the shared CLI. The duplicate block is gone. Suites: utils 185/185, mcp-register 36/36, install-ps1 7/7, onboarding 11/11. Signed-off-by: Felipe Marzochi <fmarzochi@gmail.com>
Contributor
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/lib/utils.js">
<violation number="1" location="scripts/lib/utils.js:511">
P2: PATH-based MCP detection skips a tool in the current directory when PATH contains a valid empty component. POSIX treats leading, trailing, and doubled separators as the current directory, so the fallback should preserve those entries while still distinguishing an unset PATH.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Two holes in the spawn-free fallback: a directory named like the command passed both checks, since searchable directories carry the execute bit on POSIX and simply exist on Windows, so the type is settled with statSync first. And POSIX shells read a leading, trailing or doubled separator as the current directory, which the previous filter silently dropped; those entries are honored now, while an unset or empty PATH still means there is nowhere to look. Signed-off-by: Felipe Marzochi <fmarzochi@gmail.com>
…path Second-opinion audit found a real Windows defect in the new PATH detection: with OpenCode installed but never launched, the XDG target opened on the PATH signal and wrote a config the Windows editor never reads, while the AppData target stayed shut because its file did not exist yet. The PATH signal now opens the AppData target on Windows and the XDG one everywhere else. Two smaller fixes in the PATH scan it also caught: a command passed with an explicit extension was being suffixed with PATHEXT (node.exe.EXE), and quoted Windows PATH entries kept their quotes, which broke every lookup inside them. The unit test's stand-in binary now carries a .cmd extension on Windows, where an extension-less file is not executable and the assertion would have been vacuous. Signed-off-by: Felipe Marzochi <fmarzochi@gmail.com>
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
The PATH-only branch for a never-launched OpenCode on Windows had no coverage, so it could silently regress back to writing the config the editor does not read. The case asserts the AppData gate opens and the XDG one stays shut on Windows, and the reverse everywhere else. Signed-off-by: Felipe Marzochi <fmarzochi@gmail.com>
|
This was referenced Aug 5, 2026
Merged
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
Three entry points install EGC (
egc init,install.sh,install.ps1) and each kept its own hand-written copy of which tools get the MCP servers registered. The copies had drifted, and the drift was invisible because every path prints success either way.What was actually broken
egc initon the same machine.%APPDATA%\opencodewhilemcp-register.jsused the XDG-style path.Fix
Both shells now call a thin CLI over the existing registry (
scripts/lib/mcp-register-cli.js), so there is one list and one set of format handlers (JSON, TOML, Continue YAML, Zed context servers, Claude CLI). Their hand-rolled JSON and TOML writers are deleted, which also removes the duplicated TOML escaping. The Windows AppData location for OpenCode survives as its own existence-gated target so no Windows user loses a registration that used to work.Verification
Clean machine profile with Continue.dev and Zed present: the shell installer registered both, which it never did before this change. Suites: mcp-register 34/34, install-sh 6/6, install-ps1 7/7, install-onboarding 11/11.
Note for a follow-up, found while mapping this:
egc install --target <tool>never registers MCP servers at all (onlyegc initand the shells do), yet docs/installation.md tells the reader thategc install --target zedwrites Zed's context servers.Summary by cubic
Unifies MCP registration across
egc init,install.sh, andinstall.ps1via a shared CLI. Tightens PATH detection and Windows handling (correct OpenCode AppData routing) and only reads a project.mcp.jsonfrom the working directory.Bug Fixes
commandExistswith a spawn-free PATH scan fallback; accepts only real files, honors empty PATH entries, and handles explicit extensions and quoted Windows PATH entries.USERPROFILE; support OpenCode at%APPDATA%\opencodeand route PATH-only installs there on Windows..mcp.jsonfrom the invoking CWD; exclude the package root via canonical physical path (case-insensitive on Windows); removed duplicate project merge and Claude registration in installers.Refactors
scripts/lib/mcp-register-cli.js; all entry points share one registration list and format writers; removed shell JSON/TOML writers. Installers run the CLI from the invoking directory.Written for commit 3e61f62. Summary will update on new commits.