Skip to content

fix: scope plugin enablement per project so multi-org developers get only their current repo's MCP + skills (#20)#21

Merged
byapparov merged 2 commits into
mainfrom
20-project-scope-enablement
May 20, 2026
Merged

fix: scope plugin enablement per project so multi-org developers get only their current repo's MCP + skills (#20)#21
byapparov merged 2 commits into
mainfrom
20-project-scope-enablement

Conversation

@byapparov
Copy link
Copy Markdown
Contributor

Summary

Closes #20.

Pre-#20, the installer wrote enabledPlugins["aictrl-<org>@aictrl"] = true to user-scope ~/.claude/settings.json. A developer with access to two aictrl orgs (e.g. celliq and talentrix) ran npx @aictrl/plugin in each repo and ended up with both plugins enabled globally — every Claude Code session in every project connected both MCP servers and loaded both skill catalogues, regardless of which repo they were actually in.

After this PR

installClaudePlugin writes enablement to project-scope <projectDir>/.claude/settings.local.json instead. Plugin installation (marketplace dir, plugin files, installed_plugins.json) stays global — only the enable toggle is per-project.

cd ~/code/celliq && claude     → only aictrl-celliq MCP + skills load
cd ~/code/talentrix && claude  → only aictrl-talentrix MCP + skills load

Telemetry, credentials, and the slash-command hook are unchanged — they already resolve the active org from .aictrl.json walking up the CWD.

Migration for users on a pre-#20 install

On every run, installClaudePlugin now:

  1. Writes the enablement entry to project-scope settings.local.json.
  2. Removes this org's entry from user-scope ~/.claude/settings.json if present (preserves every other entry — other orgs' aictrl plugins, claude-plugins-official plugins, etc.). The next install in another org's repo migrates that org's entry too.
  3. Adds .claude/settings.local.json to project .gitignore (it's per-developer state, shouldn't be committed).

API change

ClaudePluginOptions swaps:

-  settingsFile: string;        // path to ~/.claude/settings.json
+  projectDir: string;          // project root; settings.local.json written under here
+  userSettingsFile: string;    // ~/.claude/settings.json — read for legacy cleanup only

cli.ts now passes projectDir (already computed at the top of main()) and renames the user-settings arg.

Repro (from #20)

  1. npx @aictrl/plugin --org celliq --api-key ... in ~/code/celliq.
  2. npx @aictrl/plugin --org talentrix --api-key ... in ~/code/talentrix.
  3. Before: cd ~/code/celliq && claude connects both aictrl-celliq and aictrl-talentrix MCP servers; both skill sets visible.
  4. After: only aictrl-celliq loads.

Test plan

5 new regression tests in test/writers/claude.test.ts (suite total: 114, up from 109):

  • does not write any enablement entry to user-scope settings.json
  • removes legacy user-scope enablement on upgrade and preserves unrelated entries
  • does not touch unrelated aictrl-<other-org>@aictrl entries in user settings — celliq install must not clobber a pre-existing talentrix entry
  • adds .claude/settings.local.json to project .gitignore
  • multi-org: two project dirs each get only their own org enabled — end-to-end multi-org scenario asserting both settings.local.json files land correctly and the global marketplace still carries both plugin entries

The existing enables plugin in [project-scope] settings.local.json and preserves existing entries in project settings.local.json when merging tests were rewritten to target the new file. The end-to-end consistency test now reads enablement from the project file.

  • npm test — 114/114 pass
  • npm run build — clean tsc
  • CI green
  • Manual: install in two repos for two orgs, confirm only the current repo's plugin/MCP/skills load

🤖 Generated with Claude Code

…only their current repo's MCP + skills

Closes #20

Pre-#20 versions wrote enablement to user-scope ~/.claude/settings.json.
That meant every Claude Code session in every repo loaded *every*
installed org's MCP server and skill catalogue. A developer with access
to celliq and talentrix saw both MCPs connect and both skill sets appear
in every session, regardless of which repo they were actually in.

Move enablement to <projectDir>/.claude/settings.local.json. Plugin
installation (marketplace, plugin files, installed_plugins.json) stays
global — only the *enable* toggle is per-project.

After this commit:
  cd ~/code/celliq && claude   → only aictrl-celliq MCP + skills load
  cd ~/code/talentrix && claude → only aictrl-talentrix MCP + skills load

Telemetry, credentials, and per-project routing in the slash-command
hook are unchanged — they already key off .aictrl.json.

Migration: on install we clean this org's user-scope enablement entry
out of ~/.claude/settings.json (preserving every other entry so other
orgs and non-aictrl plugins survive). The next install in another org's
repo migrates that org's entry.

The new settings.local.json is auto-added to the project .gitignore
since it carries per-developer enablement state.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@aictrl-dev
Copy link
Copy Markdown

aictrl-dev Bot commented May 20, 2026

Now let me read the reference files and build the manifests.Now let me read the patches and build the manifests.Now I have everything. Total changed lines S = 338, so N = 1 bug hunter shard. I'll dispatch all 3 subagents (Security, Consistency, Bug Hunter 1/1) in parallel.All 3 subagents have returned. Let me merge and deduplicate the findings, then record the review.

Subagent results:

  • Security: 0 findings
  • Consistency: 1 finding
  • Bug Hunter: 3 findings

After deduplication by (file, line, rule_slug, description) — no duplicates found. Total: 4 findings, sorted by severity then file:line.---

Review: PR #21 — Scope plugin enablement per project

1 MAJOR | 3 MINOR | 0 blockers | 0 nits


Data Integrity

[MAJOR] src/writers/claude.ts:351 — Non-atomic write can corrupt user settings.json

removeUserScopeEnablement reads ~/.claude/settings.json, mutates it in-memory, then overwrites the file with writeFile. If the process is killed (SIGKILL, power loss, OOM) during the write, the file may be truncated to zero bytes, losing all user-scope Claude Code settings — not just aictrl entries. This is a user-global file, so the blast radius is high.

// current
delete (enabledPlugins as Record<string, unknown>)[pluginDirName];
await writeFile(userSettingsFile, JSON.stringify(settings, null, 2) + '\n', 'utf-8');

Fix: Write to a temp file first, then rename:

import { rename } from 'fs/promises';
// ...
const tmp = userSettingsFile + '.tmp';
await writeFile(tmp, JSON.stringify(settings, null, 2) + '\n', 'utf-8');
await rename(tmp, userSettingsFile);

Correctness

[MINOR] src/writers/claude.ts:19path.join produces backslash in gitignore on Windows

PROJECT_SETTINGS_RELPATH is computed with path.join('.claude', 'settings.local.json'), which yields .claude\settings.local.json on Windows. This value is passed to ensureGitignore, which writes it verbatim into .gitignore. Git on Linux/macOS only matches forward-slash patterns, so the entry would silently fail to ignore the file if the .gitignore is shared across platforms.

// current
const PROJECT_SETTINGS_RELPATH = join('.claude', 'settings.local.json');

Fix: Use a plain forward-slash string literal — identical result on POSIX, safe on Windows:

const PROJECT_SETTINGS_RELPATH = '.claude/settings.local.json';

Consistency

[MINOR] src/writers/claude.ts:182 — Gitignore responsibility differs across writers

installClaudePlugin now calls ensureGitignore internally, but installCursor delegates gitignore to its caller (main() in cli.ts). The two writer modules expose inconsistent contracts: one is self-contained for gitignore, the other is not. A future consumer calling installCursor directly would miss the gitignore step.

Fix: Either move ensureGitignore out of installClaudePlugin and handle both writers' gitignore entries in main() (matching the existing cursor pattern), or have installCursor also call ensureGitignore internally.


Performance / Housekeeping

[MINOR] src/writers/claude.ts:170 — Migration cleanup runs on every install

removeUserScopeEnablement is called unconditionally on every installClaudePlugin invocation. After the first post-migration run, the function reads the user settings file, walks all validation checks, finds the key absent, and returns — a redundant file-read + JSON.parse for a permanently false condition. Not a correctness bug, but worth noting.

Fix: Acceptable as-is (cost is one stat + read of a small file). If desired, add a brief comment clarifying this is intentional per-run cleanup, or gate behind existsSync(userSettingsFile) before calling.

… migration intent comment

Addresses 3 of 4 aictrl-dev review findings:

- [MAJOR] extract writeJsonAtomic helper (tmp + rename) and use it for
  both user-scope settings.json (high-blast-radius user-owned file) and
  the new project-scope settings.local.json (also carries user state
  like theme). Original file content stays intact until the new content
  is durable, so a SIGKILL/power-loss mid-write cannot truncate the file.
- [MINOR] use a forward-slash literal for PROJECT_SETTINGS_RELPATH so
  the value is correct in both the .gitignore (which only matches POSIX
  separators) and Node fs operations (Node accepts forward slashes on
  Windows too). Avoids a Windows-only bug where the gitignore entry
  would silently fail to match the actual file.
- [MINOR] comment clarifying that removeUserScopeEnablement runs every
  install intentionally — self-healing if a stale entry returns via
  backup restore or manual edit.

The 4th finding (gitignore-responsibility-differs-across-writers) is
deferred to #22 — the principled fix harmonises in the opposite
direction the reviewer suggested (move cursor's gitignore call into
installCursor) which is broader scope than #21.

2 new regression tests; suite 116/116 (was 114). Build clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@byapparov
Copy link
Copy Markdown
Contributor Author

Review response — PR #21

All 4 findings verified TRUE. 3 fixed in-PR, 1 deferred to a follow-up issue (it would regress test coverage to fix here).

Issues addressed (pushed to this PR)

  • [MAJOR] Non-atomic write to user ~/.claude/settings.jsonsrc/writers/claude.ts:351: extracted a writeJsonAtomic helper that writes to <file>.tmp and renames atomically, then routed both removeUserScopeEnablement (user-global file) and mergeSettings (project settings.local.json — also carries user-owned state like theme) through it. Original file content is preserved until the new content is fully durable.
  • [MINOR] path.join produces backslash on Windowssrc/writers/claude.ts:19: changed PROJECT_SETTINGS_RELPATH from join('.claude', 'settings.local.json') to the literal '.claude/settings.local.json'. Node's path API accepts forward slashes on Windows for filesystem operations, and .gitignore only matches POSIX separators, so this is the single value that's correct in both contexts.
  • [MINOR] Migration cleanup runs every install (undocumented intent)src/writers/claude.ts:170: added a comment clarifying the unconditional call is intentional (self-healing if a stale entry returns via backup restore or manual edit; cost is one small file read).

2 regression tests added in test/writers/claude.test.ts:

  • writes user-scope settings.json atomically (no .tmp file remains on success) — verifies the happy-path leaves no temp file behind and the final content is correct.
  • survives a pre-existing user settings.json with a stale .tmp sibling — verifies the atomic-write pattern doesn't trip over a leftover .tmp from a previously-crashed run.

The existing adds .claude/settings.local.json to project .gitignore test was tightened to also assert the file contains no backslash anywhere — catches the Windows regression directly.

Suite: 116/116 pass (was 114). npm run build clean.

Review claims verified false (no change needed)

None — all 4 findings were verified true.

Not addressed here

@byapparov byapparov merged commit 30426e7 into main May 20, 2026
5 checks passed
@byapparov byapparov deleted the 20-project-scope-enablement branch May 20, 2026 18:30
@byapparov byapparov mentioned this pull request May 20, 2026
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Multi-org developers get both MCPs/skills loaded in every project (no per-repo isolation)

1 participant