chore(install): stop shipping personal tooling in the package - #1202
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 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.
1 issue found across 10 files
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="tests/lib/mcp-register.test.js">
<violation number="1" location="tests/lib/mcp-register.test.js:109">
P3: The two new Windows-routing tests mutate `process.platform` at runtime, but utils.js captured `isWindows` as a module-load-time constant, so on the Linux test runner `commandExists`/`existsOnPath` always take the POSIX branch. The tests consequently never exercise the Windows PATHEXT/.cmd resolution logic that this PR adds in `existsOnPath` - the 'AppData routing' test passes only because the fixture's extension-less `opencode` file is found by the POSIX `which` branch. Consider driving the Windows code path directly (e.g. factoring the platform-dependence into a parameter/argument that the tests can inject) so the new Windows executable search is actually covered rather than passing incidentally.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
1 issue found across 5 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="tests/scripts/install-sh.test.js">
<violation number="1" location="tests/scripts/install-sh.test.js:162">
P2: This test executes the real mcp-register-cli.js against the live host environment rather than a hermetic one: all PATH-gated MCP targets fire on whatever is installed (claude via `claude mcp add -s user`, plus kiro/codex/opencode/cursor via commandExists). That makes the output and actual coverage vary by machine and invokes real third-party binaries, unlike the deterministic source check it replaces. Consider gating the CLI run or the env (e.g., a minimal PATH that still lets `node` resolve, and stubbing `commandExists`/`resolveClaudeCli`) so it only exercises the Cursor skip the test targets.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Three things in the published package belonged to one person's workstation, not to EGC: - mcp-configs/ travels in the npm tarball via package.json files, and its catalog carried an 'obsidian' entry pointing at a local REST plugin with a bearer token, plus a 'devfleet' entry pointing at localhost:18801. Neither is an EGC component; both are gone. - Both installers ran an Obsidian propagation step: they read whatever Obsidian MCP config they could find in the user's Antigravity, Gemini or Cursor files and copied it into every other tool's config. The installer has no business writing a third-party integration into somebody's editors, and the whole routine is removed. The Python vault providers stay: they are generic, take vaultPath from configuration, embed no path of their own, and only run when somebody deliberately asks for them. Anyone who wants that integration configures it themselves. The install.sh assertion that policed the 'not valid JSON' skip note now checks the registration CLI, which is where the refusal lives after the parity work. Suites: install-sh 6/6, install-ps1 7/7, onboarding 11/11. Signed-off-by: Felipe Marzochi <fmarzochi@gmail.com>
…ows coverage
Four review findings:
- commandExists('tool.exe') appended PATHEXT on top of a name that already carried an extension, so tool.exe.CMD would report a match the shell would never resolve. A name with an extension is now looked up exactly as written.
- Push-Location treated the invoking directory as a wildcard pattern, so a real directory containing [ ] * or ? aborted the installer before registration and everything after it. -LiteralPath fixes that.
- The install.sh assertion I had weakened to a string match is now behavioral: a broken Cursor config in a throwaway HOME must produce a skip note, must not be reported as registered, and must come back byte-for-byte unchanged.
- The Windows-routing tests stubbed process.platform, but utils.js had captured isWindows at module load, so they silently took the POSIX branch and proved nothing. commandExists and existsOnPath read the platform at call time now. That exposed two real artifacts of running Windows rules on a case-sensitive Linux filesystem: PATHEXT casing (pinned in the test) and the PATH separator (path.delimiter, which is already ';' in any real Windows process).
Suites: mcp-register 37/37, utils 185/185, install-sh 6/6, install-ps1 7/7, onboarding 11/11.
Signed-off-by: Felipe Marzochi <fmarzochi@gmail.com>
Only the Obsidian entry was somebody's personal setup. The devfleet entry describes an orchestration server the catalog legitimately points at, so it stays. Signed-off-by: Felipe Marzochi <fmarzochi@gmail.com>
The assertion ran the real registration CLI against the host environment, so every PATH-gated target fired on whatever happened to be installed, including 'claude mcp add -s user'. That mutated the machine running the suite and made both the output and the coverage vary by host. An empty PATH closes every PATH-gated target, leaving only the seeded Cursor config, which is the one under test; process.execPath runs the CLI directly, so node needs no PATH entry. Verified: 6/6, and the developer machine's own Cursor config is untouched. Signed-off-by: Felipe Marzochi <fmarzochi@gmail.com>
The directory was removed only after a successful run, so a CLI that exited nonzero or timed out left it behind: execFileSync throws and the line never ran. Both temp dirs are removed in the finally block now. Signed-off-by: Felipe Marzochi <fmarzochi@gmail.com>
Fmarzochi
force-pushed
the
chore/remove-personal-tooling-from-package
branch
from
August 5, 2026 19:49
e1aaec8 to
5001caf
Compare
|
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.
What was wrong
Three things in the published package described one particular workstation rather than EGC.
1. The MCP catalog shipped somebody's Obsidian and a private orchestrator.
mcp-configs/travels in the npm tarball (it is listed inpackage.jsonfiles), and the catalog carried anobsidianentry pointing at a local REST plugin with a bearer token, plus adevfleetentry pointing atlocalhost:18801. Neither is an EGC component.2. Both installers propagated Obsidian into other tools.
install.shandinstall.ps1searched the user's Antigravity, Gemini and Cursor configs for an Obsidian MCP block and copied it into every other tool's config. An installer has no business writing a third-party integration into somebody's editors, so the whole routine is gone (86 lines of shell, 49 of PowerShell).What deliberately stays
The Python vault providers (
src/llm/memory/providers/obsidian.py,mcp_obsidian.py). They are generic, takevaultPathfrom configuration, embed no path of their own, and only run when somebody explicitly selects that provider. Anyone who wants that integration sets it up themselves, which is the point.Verification
Suites: install-sh 6/6, install-ps1 7/7, install-onboarding 11/11. The install.sh assertion that policed the "not valid JSON" skip note now checks the registration CLI, which is where that refusal lives after the registration parity work.
Depends on nothing; stacks cleanly after #1197.
Summary by cubic
Removes the Obsidian MCP entry and stops Obsidian propagation. Unifies MCP registration across shells and
egc initvia a shared CLI with safer project config handling; keepsdevfleetin the catalog and tightens Windows path/extension handling.Refactors
obsidianfrom the published MCP catalog; keepdevfleet.install.shandinstall.ps1.scripts/lib/mcp-register-cli.js; installers delegate to it and run from the invoking directory to merge a project.mcp.json(CWD-only). Excludes the package root; usesUSERPROFILEon Windows.Bug Fixes
egc init(adds Continue.dev and Zed).%APPDATA%, avoid double Claude registration, and treat the invoking path literally (Push-Location -LiteralPath).PATHEXT), accepts only real files, and handles quoted PATH entries and empty segments.Written for commit 5001caf. Summary will update on new commits.