fix: register aictrl marketplace so Claude Code can resolve plugin enablement (#18)

[byapparov]

Summary

Closes #18.

@aictrl/plugin was writing the plugin into ~/.claude/plugins/cache/<plugin>@aictrl/ and flipping settings.enabledPlugins["<plugin>@aictrl"] = true, but it never registered the aictrl marketplace with Claude Code. The loader couldn't resolve the enablement entry and printed Plugin "<plugin>" not found in marketplace "aictrl" on every session.

Root cause

Claude Code needs three index files to resolve a <plugin>@<marketplace> enablement:

1. ~/.claude/plugins/known_marketplaces.json — registers the marketplace
2. ~/.claude/plugins/marketplaces/<name>/.claude-plugin/marketplace.json — declares the plugin
3. ~/.claude/plugins/installed_plugins.json — records where the install lives

The old installer wrote (0) the on-disk plugin and (4) the enabledPlugins toggle, but skipped (1)–(3). That orphaned every install.

Fix

Switch to the canonical Claude Code layout. Plugin files now live at:

~/.claude/plugins/marketplaces/aictrl/
  .claude-plugin/marketplace.json        ← declares plugin (source: ./plugins/aictrl-<orgSlug>)
  plugins/aictrl-<orgSlug>/
    .claude-plugin/plugin.json
    .mcp.json
    hooks/...
    skills/...

The installer also merges entries into known_marketplaces.json (source=local) and installed_plugins.json. The enabledPlugins key shape (<plugin>@aictrl) is unchanged, so settings written by older versions still resolve after upgrade.

Cleanup: on re-install we delete the orphan ~/.claude/plugins/cache/<plugin>@aictrl/ directory left by pre-fix versions so upgraders don't end up with stale state.

Hook update: slash-command-telemetry.sh now searches both ~/.claude/plugins/cache and ~/.claude/plugins/marketplaces for SKILL.md files, since canonical plugins live in the second location.

Repro (from #18)

1. Run npx @aictrl/plugin for any org.
2. Open Claude Code anywhere.
3. Before: banner reads Plugin "aictrl-<orgSlug>" not found in marketplace "aictrl" every session.
4. After: marketplace + install metadata is written, banner is gone, skills load normally.

Test plan

Unit tests added in test/writers/claude.test.ts (6 new tests, 103 total — up from 97):

• writes marketplace manifest declaring the plugin
• registers the marketplace in known_marketplaces.json
• preserves existing marketplaces when registering aictrl
• records the install in installed_plugins.json
• preserves installedAt across re-installs but updates lastUpdated
• cleans up legacy cache/<plugin>@aictrl/ directory from older installs

Plus the existing 8 installClaudePlugin tests were updated to the new on-disk paths, and one assertion in test/hooks/claude-slash.test.ts was widened to match the broader find root.

• npm test — 103/103 pass
• npm run build — clean tsc
• CI green
• Manual: npx @aictrl/plugin against a real org, open Claude Code, confirm no banner

🤖 Generated with Claude Code

[aictrl-dev]

Now I have all the patches. Let me build the manifests and dispatch subagents.

Manifests:

• Security: src/writers/claude.ts, src/config.ts, src/hooks/claude-slash.sh.ts (filesystem writes to user home, plugin trust boundaries, shell generation)
• Consistency: all 7 text files
• Bug Hunter: src/cli.ts, src/config.ts, src/hooks/claude-slash.sh.ts, src/writers/claude.ts, test/hooks/claude-slash.test.ts, test/writers/claude.test.ts (S=406, N=1 shard)Now let me merge and deduplicate the findings from all three subagents, then record them.

Deduplication:

• existsSync guard issue reported by Security (line 48), Consistency (line 2), and Bug Hunter (line 50) — keeping the Security TOCTOU angle (most impactful) and the Consistency import note
• CLAUDE_PLUGINS_CACHE dead export reported by Consistency and Bug Hunter — keeping the MINOR version
• Total: 7 unique findingsLet me re-record the three findings that failed due to summary length:Now let me finalize the review:## Code Review: PR fix: register aictrl marketplace so Claude Code can resolve plugin enablement (#18) #19 — Marketplace Registration Fix

1 MAJOR · 5 MINOR · 1 NIT

---

MAJOR

src/writers/claude.ts:47 — Unsanitized orgSlug in recursive rm path-traversal footgun

The legacy-cleanup block constructs legacyCacheDir from pluginDirName which is derived from orgSlug without path-traversal validation. Because path.join resolves .. segments, an orgSlug containing /../ would cause rm(..., { recursive: true, force: true }) to delete an arbitrary directory outside the plugin tree. While this is a local CLI tool (the user already has filesystem access), it's still a footgun: a malformed orgSlug could unintentionally delete important directories.

const legacyCacheDir = join(pluginsRoot, 'cache', pluginDirName);
if (existsSync(legacyCacheDir)) {
    await rm(legacyCacheDir, { recursive: true, force: true });

Suggested fix: Validate orgSlug at the top of installClaudePlugin (e.g. reject slashes, .., and null bytes: if (/\/|\.\.|\0/.test(orgSlug)) throw new Error('Invalid orgSlug');). Alternatively, resolve the path and confirm it stays under pluginsRoot before calling rm.

---

MINOR

src/writers/claude.ts:48 — Redundant existsSync creates TOCTOU window before forced rm

rm(..., { force: true }) already ignores missing paths, making the preceding existsSync check redundant. The check-then-act pattern introduces a TOCTOU race: the directory could be replaced by a symlink between existsSync and rm. It also blocks the event loop with a synchronous fs call.

Suggested fix: Remove the if (existsSync(legacyCacheDir)) guard and call await rm(legacyCacheDir, { recursive: true, force: true }) directly. Remove the existsSync import from 'fs'.

---

src/writers/claude.ts:173 — JSON array pollutes manifest via object spread

typeof [] === 'object' is true in JS. If marketplace.json is a JSON array, { ...manifest, ...parsed } spreads numeric-indexed properties into the object, polluting the manifest with keys like "0", "1" that persist across writes.

      manifest = { ...manifest, ...parsed };
      if (!Array.isArray(manifest.plugins)) manifest.plugins = [];

Suggested fix: Guard with && !Array.isArray(parsed) before spreading: manifest = { ...manifest, ...(Array.isArray(parsed) ? {} : parsed) };

---

src/writers/claude.ts:204 — JSON array accepted as object in mergeKnownMarketplace

Same typeof [] === 'object' issue. If known_marketplaces.json is corrupted into a JSON array, assigning data[MARKETPLACE_NAME] = {...} adds a named property to the array, which JSON.stringify silently drops, losing the marketplace registration on the next write.

    if (parsed && typeof parsed === 'object') {
      data = parsed;

Suggested fix: Change the guard to if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) data = parsed;

---

src/writers/claude.ts:239 — Reinstall replaces entire plugin entry array

mergeInstalledPlugin reads only data.plugins[pluginKey]?.[0] and overwrites the entire array with a single { scope: 'user' } entry. If Claude Code stores multiple scope entries for the same plugin (e.g., both 'user' and 'project'), re-running the installer would discard the non-user entries.

Suggested fix: Merge instead of replace: find the 'user'-scope entry in the existing array, update it in place, and preserve sibling entries from other scopes.

---

src/config.ts:10 — CLAUDE_PLUGINS_CACHE exported but no longer imported

src/cli.ts was the sole consumer and now imports CLAUDE_PLUGINS_ROOT instead. CLAUDE_PLUGINS_CACHE is still exported but may have no remaining consumers.

Suggested fix: Remove the CLAUDE_PLUGINS_CACHE export unless it is part of the published API surface. If it must be kept for backward compatibility, mark it @deprecated.

---

NIT

src/writers/claude.ts:2 — Synchronous fs import in otherwise async module

The existsSync import from 'fs' introduces a synchronous filesystem call into an otherwise fully async (fs/promises) module. Combined with the redundancy noted above (rm with force: true handles missing paths), the import can be removed entirely once the guard is dropped.

[byapparov]

Review response — PR #19

All 7 findings verified TRUE and all fixed in this PR (no defers, no false positives).

Issues addressed (pushed to this PR)

• [MAJOR] Unsanitized orgSlug flows into recursive rm — src/writers/claude.ts:47: added an ORG_SLUG_REGEX = /^[a-z0-9][a-z0-9-]{0,62}$/ guard at the top of installClaudePlugin that throws on .., /, \, whitespace, uppercase, or empty strings before any filesystem write or rm. Validation runs once and covers every downstream use of orgSlug (paths, URLs, MCP server name, hooks), not just the legacy cleanup.
• [MINOR] Redundant existsSync + TOCTOU + sync I/O — src/writers/claude.ts:48: dropped the if (existsSync(...)) guard, relying on rm({recursive:true, force:true}) which is a no-op when the target is missing. Removed the existsSync import from 'fs', leaving the module fully async (also resolves the NIT below).
• [MINOR] Array-shaped manifest pollutes via object spread — src/writers/claude.ts:173: guard widened to parsed && typeof parsed === 'object' && !Array.isArray(parsed) before merging the existing manifest. Prevents numeric-indexed keys leaking into the rewritten file.
• [MINOR] Same array-as-object issue in mergeKnownMarketplace — src/writers/claude.ts:204: same !Array.isArray guard added, so a corrupt array-shaped known_marketplaces.json is treated as "start fresh" rather than having the aictrl entry silently dropped by the next JSON.stringify pass. Same defensive guard added to mergeInstalledPlugin for symmetry.
• [MINOR] Re-install replaces entire entry array, losing non-user scopes — src/writers/claude.ts:239: mergeInstalledPlugin now partitions existing entries by scope, only replaces the scope: 'user' row, and re-emits the array with any non-user entries preserved. The installer only writes user-scope, so foreign scopes survive a re-install.
• [MINOR] CLAUDE_PLUGINS_CACHE exported but unused — src/config.ts:10: removed. The only previous consumer (src/cli.ts) already switched to CLAUDE_PLUGINS_ROOT in the original commit, so the export had no callers.
• [NIT] Sync fs import in async module — src/writers/claude.ts:2: subsumed by the second fix above; the import is gone.

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

• rejects orgSlug containing path-traversal sequences
• legacy cleanup is a no-op when the cache dir does not exist
• does not pollute marketplace.json when the existing file is a JSON array
• does not lose the aictrl entry when known_marketplaces.json is a JSON array
• preserves non-user-scope entries in installed_plugins.json across re-install

Suite: 108/108 pass (was 103). npm run build clean.

Review claims verified false (no change needed)

None — all 7 findings were verified true and worth fixing.

Not addressed here

None — every finding is in scope and applied to this branch.