feat: add cursor-scope prompt for project vs global install#60
feat: add cursor-scope prompt for project vs global install#60adamgomez-ux wants to merge 3 commits intojulianoczkowski:mainfrom
Conversation
julianoczkowski
left a comment
julianoczkowski
left a comment
There was a problem hiding this comment.
Thanks for the contribution, Adam! The idea is useful and the project-scope path is clean. I checked out the branch locally, ran the full test suite, and scaffolded all three templates (React, Angular, SolidJS) against both scopes. A few things to call out:
What works well
- All 54 tests pass on the PR branch.
--helpcorrectly shows the new--cursor-scopeflag and invalid values are rejected with a clear error.- Dry-run output for all three frameworks includes the new
Cursor config:line. - Project-scope (default) scaffolding is unchanged end-to-end for React / Angular / SolidJS including
npm install+ post-scaffold validation. No regression for existing users. - Global-scope scaffolding mechanically does the right thing:
.cursor/is omitted from the project folder and written to~/.cursor/.
What needs to change before merge (two are destructive)
- Critical – the new test silently overwrites the developer's real
~/.cursor/mcp.json. I verified this firsthand: my personal~/.cursor/mcp.jsonwent from 826 bytes (my actual MCP servers) to 231 bytes (the React template's minimal file) just from runningnpm test. Any maintainer or CI runner using Cursor will lose their global MCP config. See the inline comment ontests/utils/git.test.js. - Critical – the production
globalpath has the same overwrite problem for real end users.copyDirectoryusesfs.copyFile, which overwrites unconditionally. A user who picks "Global" in the prompt will lose any existing~/.cursor/mcp.jsonand may have~/.cursor/commands,~/.cursor/rules,~/.cursor/skillsfiles clobbered on name collision. See the inline comment onsrc/utils/git.js. - Design – running global scope for multiple frameworks pollutes
~/.cursor/rules/. Rule filenames are framework-specific (e.g.modus-react-master.mdc,modus-angular-master.mdc), so picking global scope for React and later for Angular leaves both sets active on every project on the machine. Worth documenting or namespacing. - Correctness – Cursor's "Rules for AI" and slash-commands are primarily project-scoped. Copying
rules/andcommands/into~/.cursor/does not make Cursor apply them globally the way the prompt hint implies. Only~/.cursor/mcp.jsonand (partially)~/.cursor/skills/are actually picked up globally. Prompt wording should be tightened. - Nit – step-number comments in
src/scaffold.jsgot desynchronized (jumps 2 → 4 → 5 → 6 → 7 → 8 → 9 → 9). Functional only; cosmetic fix. - Docs – README CLI options table and the
--helpExamples footer don't mention--cursor-scopeyet. You flagged the README one yourself in the PR body. - Nit –
.claude/settings.local.jsonis a personal dev-env artifact; broadening the bash allow-list here probably shouldn't be part of this PR (and arguably the file should be.gitignored with an example committed).
Happy to help with a follow-up if it's easier. Marking as Request Changes primarily for #1 and #2 — those will bite real users and contributors.
| await copyTemplate("react", projectDir, { cursorScope: "global" }); | ||
|
|
||
| // ~/.cursor/ should now contain mcp.json from the template | ||
| expect(existsSync(join(globalCursorPath, "mcp.json"))).toBe(true); |
There was a problem hiding this comment.
Critical – this test overwrites the developer's real ~/.cursor/mcp.json.
I ran npm test on a clean checkout of this branch and my personal ~/.cursor/mcp.json went from 826 bytes (my actual MCP server config) to 231 bytes (the template's minimal 2-server file). Every maintainer, contributor, or CI job with a Cursor install will silently lose their global MCP config when running the test suite.
The root cause is that copyTemplate(..., { cursorScope: "global" }) resolves homedir() to the real home dir with no test isolation.
Suggested fix – inject an overridable global path and mock it in the test:
// in src/utils/git.js signature
export async function copyTemplate(
templateName,
targetPath,
{ cursorScope = "project", globalCursorPath } = {}
) { ... }
// in the global branch
const target = globalCursorPath ?? path.join((await import("os")).homedir(), ".cursor");
await copyDirectory(cursorSrc, target);Then in this test, pass a temp dir:
const fakeHome = join(tmpdir(), "cta-fake-home-" + Date.now());
await copyTemplate("react", projectDir, {
cursorScope: "global",
globalCursorPath: join(fakeHome, ".cursor"),
});
expect(existsSync(join(fakeHome, ".cursor", "mcp.json"))).toBe(true);
// cleanup fakeHome in finallyAlternatively vi.spyOn(os, "homedir") to redirect it, but the injected-path approach is simpler and also useful for the real code path (e.g. future --cursor-global-path flag).
| const { homedir } = await import("os"); | ||
| const cursorSrc = path.join(bundledPath, ".cursor"); | ||
| const globalCursorPath = path.join(homedir(), ".cursor"); | ||
| await copyDirectory(cursorSrc, globalCursorPath); |
There was a problem hiding this comment.
Critical – this is destructive to the end user's existing Cursor config.
copyDirectory ultimately calls fs.copyFile, which overwrites unconditionally. A real user who picks the "Global" option in the interactive prompt will silently lose:
~/.cursor/mcp.json– their personal MCP servers (confirmed on my own machine via the test).- Any files in
~/.cursor/commands/,~/.cursor/rules/,~/.cursor/skills/that happen to share a filename with the template.
At minimum I'd suggest one or more of:
- Back up before overwrite: rename any existing
~/.cursor/mcp.jsontomcp.json.bak-<ISO-timestamp>before writing. - Merge strategy for
mcp.json: read existing JSON, mergemcpServerskeys (template wins on collision but existing keys are preserved), then write. This is what most users will actually want. - Skip-if-exists for
rules/,commands/,skills/: only copy files that don't already exist, unless the user passes--force. - Confirmation prompt: if any of those targets already exist, show a second
p.confirmwith a list of files that will be affected before proceeding.
(1) + (3) is the minimum bar for "not destructive". (2) is the nicest UX.
| await copyDirectory(bundledPath, targetPath); | ||
| if (cursorScope === "global") { | ||
| // Copy project files, skipping .cursor/ (it goes to ~/.cursor/ instead) | ||
| const projectSkipDirs = new Set([...SKIP_DIRECTORIES, ".cursor"]); |
There was a problem hiding this comment.
Minor – consider also skipping .cursor in the recursive pass if any template ever nests it (defensive; current templates don't). More importantly, a doc comment here clarifying why .cursor is excluded would help future readers — something like:
// Global scope: template's .cursor/ is redirected to ~/.cursor/ below,
// so we don't also want it inside the project folder.
const projectSkipDirs = new Set([...SKIP_DIRECTORIES, ".cursor"]);| { | ||
| label: "Global (~/.cursor/)", | ||
| value: "global", | ||
| hint: "Active across all your Cursor projects", |
There was a problem hiding this comment.
The hint text "Active across all your Cursor projects" is only accurate for ~/.cursor/mcp.json and (partially) ~/.cursor/skills/. Cursor does not apply ~/.cursor/rules/*.mdc as global rules — Rules for AI are project-scoped; the global equivalent is the user-settings "Rules" field. Similarly ~/.cursor/commands/ is not a supported global commands dir at the moment.
Two options:
- Narrow what gets copied in global mode to just
mcp.json(and maybeskills/) and tighten the hint to"Installs global MCP config (~/.cursor/mcp.json)". - Keep copying everything but soften the hint, e.g.
"Installs Modus MCP globally; rules/commands become available to Cursor if/where it reads them".
Option 1 is probably the honest one, since Rules really are meant to travel with the project.
| } | ||
| } | ||
|
|
||
| // 4. Installation Location Choice |
There was a problem hiding this comment.
Nit – comment numbering got out of sync after inserting the new step. The current sequence reads // 2. (framework, implicit) → // 2. (this new scope prompt) → // 4. (here) → // 5. → // 6. → // 7. → // 8. → // 9. (appears twice, on L253 and L256). Purely cosmetic, but worth renumbering to 1..9 while the change is small.
| // 8. Detailed next steps | ||
| logger.nextSteps(projectName, config.name, install, installInCurrentFolder); | ||
| // 9. Detailed next steps | ||
| logger.nextSteps(projectName, config.name, install, installInCurrentFolder, installScope); |
There was a problem hiding this comment.
Duplicate // 9. comment (also on line 253). Cosmetic.
| "Bash(git push:*)", | ||
| "Bash(xargs cat *)", | ||
| "Bash(npm test *)", | ||
| "Bash(npx vitest *)", |
There was a problem hiding this comment.
Nit – .claude/settings.local.json is a personal dev-env file and this broadens the bash allow-list (xargs cat *, npm test *, npx vitest *, npm install *). These shouldn't really be shipped as part of a feature PR.
Longer term, I'd suggest .gitignore-ing .claude/settings.local.json and committing a .claude/settings.example.json instead, so everyone's local tweaks stay local.
| "--cursor-scope <scope>", | ||
| "Where to install Cursor config: project or global (default: prompt)", | ||
| validateCursorScope, | ||
| ) |
There was a problem hiding this comment.
Could you also add a --cursor-scope example to the Examples section of the help footer (the boxen block starting around line 66)? The flag is listed in Options but not demonstrated, which makes it easy to miss. Something like:
# Install Cursor config globally (for all projects on this machine)
npx @julianoczkowski/create-trimble-app@latest my-app -f react --cursor-scope global
- Fix global test to use injected globalCursorPath (fake home) instead of real ~/.cursor/ - Add globalCursorPath param to copyTemplate; global mode now only copies mcp.json and skills/ (non-destructive with backup) - Add global-scope warning in scaffold.js about multi-framework side effects - Fix global scope hint to "Installs global MCP config (~/.cursor/mcp.json)" - Fix dry-run cursor config message for global scope - Renumber scaffold.js step comments 1-9 with no duplicates - Add --cursor-scope global example to CLI help - Remove .claude/settings.local.json from git tracking; add to .gitignore; add settings.example.json Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Thanks for the thorough review Julian! I've pushed a follow-up commit addressing all 7 points: Critical #1 — Fixed global test to use an injected temp directory instead of real All 54 tests pass and all 6 dry-runs (React, Angular, SolidJS × project + global) complete cleanly. |
|
Hi Julian, I just wanted to flag that the follow-up commit addressing all 7 review items (839027e) was pushed on Apr 21. The recent upstream merge may have affected how the commits display in the PR diff. All fixes are in place: injected fakeHome temp dir for test isolation, non-destructive global copy with timestamp backup, warning for multi-framework pollution, global scope now only copies mcp.json and skills/, step comments renumbered 1–9, --cursor-scope global example added to help footer, and settings.local.json removed from git with settings.example.json committed in its place. Happy to rebase or squash if that helps the review. Thanks! |
2 participants
Pull Request
📝 Description
Adds a new
--cursor-scopeoption that lets users choose whether Cursor config (MCP, Rules, Skills) is installed at the project level or globally..cursor/is copied into the project folder — isolated, team-friendly, committable to source control.cursor/is copied to~/.cursor/— available across all projects on the machine🔗 Related Issue
N/A
🎯 Type of Change
🧪 Testing
All 54 existing tests pass. 4 new tests added to
tests/utils/git.test.jscovering:.cursor/inside the project folder.cursor/from the project folder.cursor/to~/.cursor/Manual Testing Checklist
npm run devornode bin/create-trimble-app.js)--cursor-scope projectand--cursor-scope global)Framework Testing
Tested scaffolding for:
Environment Testing
Tested on:
Node.js Version Testing
📸 Screenshots
Before
Interactive prompt went straight from framework selection to project name.
After
A new step appears after framework selection:
Success message now includes:
🔧 CLI Functionality
--cursor-scope <project|global>flag parsed properly📋 Code Quality
🔄 Breaking Changes
No breaking changes. Project scope is the default, so existing behavior is preserved for all current users.
📚 Documentation
--cursor-scopeflag appears in--helpoutput--cursor-scopeto the CLI options table💬 Additional Notes
This feature was motivated by teams wanting to commit
.cursor/config to source control so all team members get the same MCP, Rules, and Skills automatically when they clone the repo — without affecting their global Cursor setup on other projects.The
--cursor-scopeflag also makes this scriptable for automated project setup workflows.📝 Reviewer Instructions
Pull and test locally:
Test the new cursor-scope prompt:
node bin/create-trimble-app.js— select a framework, then choose scopenode bin/create-trimble-app.js my-app --framework react --cursor-scope project --dry-runnode bin/create-trimble-app.js my-app --framework react --cursor-scope global --dry-runRun tests:
npm testBy submitting this PR, I confirm that: