Skip to content

feat(skills): add per-profile skills management#711

Draft
cooco119 wants to merge 1 commit into
kaitranntt:devfrom
cooco119:feat/per-profile-skills
Draft

feat(skills): add per-profile skills management#711
cooco119 wants to merge 1 commit into
kaitranntt:devfrom
cooco119:feat/per-profile-skills

Conversation

@cooco119

@cooco119 cooco119 commented Mar 8, 2026

Copy link
Copy Markdown

Summary

  • Add skills_mode (shared | isolated) to account config, enabling per-profile skill isolation
  • Implement syncSkills() in SharedManager that converts the instance skills/ symlink to a real directory with individually symlinked shared skills
  • Add ccs skills <profile> <command> CLI that wraps npx skills with CLAUDE_CONFIG_DIR targeting the correct instance directory
  • Auto-isolate on ccs skills <profile> add, sync shared skills during ccs sync

How it works

When skills_mode: isolated is set for an account profile:

  1. The instance's skills/ symlink (→ shared → ~/.claude/skills) is replaced with a real directory
  2. Each shared skill from ~/.claude/skills/ is individually symlinked into the instance directory
  3. npx skills add -g runs with CLAUDE_CONFIG_DIR pointing to the instance, so new skills install directly into the isolated directory
  4. Profile-specific skills coexist with shared skill symlinks

Changed files

File Change
src/auth/account-context.ts AccountSkillsMode type, skillsMode in policy/metadata
src/config/unified-config-types.ts skills_mode on AccountConfig
src/types/config.ts skills_mode on ProfileMetadata
src/management/shared-manager.ts syncSkills() + syncSharedSkillsToIsolated()
src/management/instance-manager.ts Call syncSkills() in ensureInstance()
src/commands/sync-command.ts Sync isolated skills during ccs sync
src/commands/skills-command.ts New — CLI wrapping npx skills
src/ccs.ts Route skills subcommand
src/commands/help-command.ts Per-Profile Skills help section
tests/unit/skills-sync.test.ts New — 7 tests for syncSkills

Test plan

  • bun run format — clean
  • bun run typecheck — no errors
  • bun run lint:fix — clean
  • bun run validate — exit 0 (pre-existing failures unrelated)
  • bun run validate:ci-parity — passed (pre-push hook)
  • 7 new tests pass (skills-sync.test.ts)
  • 34 related tests pass (account-context, shared-context-policy, shared-manager, sync-command)
  • Manual: ccs skills personal isolateccs skills personal add <pkg>ccs skills personal list
  • Manual: ccs skills personal share restores shared mode
  • Manual: ccs sync preserves isolated skills

🤖 Generated with Claude Code

Enable per-profile skill isolation by leveraging CLAUDE_CONFIG_DIR.
When skills_mode is set to 'isolated', the instance's skills symlink is
replaced with a real directory where shared skills are individually
symlinked and profile-specific skills can coexist.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@cooco119
cooco119 marked this pull request as draft March 8, 2026 22:43
@kaitranntt kaitranntt added enhancement New feature or request area:cli-runtime CLI runtime commands, execution flow, and core target behavior labels Mar 27, 2026

@kaitranntt kaitranntt left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution! The feature direction is reasonable (per-profile skills). I reviewed and found a couple of items that should be addressed before this can land. Full report attached.

🔴 Blockers (must fix)

1. Shell injection via spawn(..., { shell: true }) with user-supplied argssrc/commands/skills-command.ts:55

The current spawn('npx', [...userInput], { shell: true }) lets shell metacharacters in user-supplied skill names execute against the host. For example:

ccs skills personal add 'foo; rm -rf ~'

would interpret the ; as a command separator. Please switch to spawn('npx', [...args]) without shell: true, or sanitize/validate args strictly before passing.

2. Destructive share modesrc/management/shared-manager.ts:585

The share operation rm -rfs the profile-only skills directory with only a single warning line. There's no confirmation prompt, no dry-run, no backup. Switching modes is reversible from a UX standpoint, but the data deletion is not. Please add either:

  • An interactive confirmation prompt (consistent with how other CCS destructive ops work), or
  • A --dry-run mode that lists what would be deleted, or
  • An automatic backup to a timestamped path before deletion.

🟡 Should address

3. Dashboard parity missing — CCS rule (CLAUDE.md "Feature Interface Requirements"): configuration features must work in both CLI and Dashboard. The new skills_mode toggle only has a CLI surface. Please add a Dashboard control, or add a follow-up issue and reference it here.

4. Windows symlink fallback untested for the per-skill case — CCS uses ~/.ccs/shared/ symlinks on Unix and copies on Windows. The new per-profile model needs to handle both. Please add Windows-path test coverage or document the behavior.

5. Cross-profile remove symlink behavior — When removing a per-profile skill that's actually a symlink into ~/.claude/, please verify the deletion does not cascade into the shared source. Add an explicit test for this case.

✅ What looks good

  • No new dependencies in package.json / ui/package.json
  • No new network calls, no token reads, no eval / base64 obfuscation
  • ~/.claude/ accessed read-only via realpath enumeration
  • Type changes are backward-compatible (new field optional, validators updated)
  • getCcsDir() used correctly (no os.homedir() direct)
  • ASCII-only CLI output, TTY-aware UI helpers
  • 7 tests cover the shared↔isolated boundary including profile-skill preservation

Architectural note (non-blocking)

This PR diverges slightly from the existing model where commands/agents/skills are all SHARED-only via ~/.ccs/shared/. By making skills opt-in per-profile, we introduce an exception. The default stays shared so it's backward compatible — but a brief design note in the PR body explaining why skills get this special treatment (or generalizing to a resources_mode for all three resource types) would help future maintainers.

Happy to re-review once the two blockers are addressed.

@kaitranntt

Copy link
Copy Markdown
Owner

Following up, since dev has moved on and this needs a refresh before it can land.

  1. Rebase needed. dev split SharedManager into smaller modules under src/management/shared-manager/. This branch still patches the old monolithic class (this.getLstat, this.linkDirectoryWithFallback, this.sharedDir and friends), so it no longer applies or compiles. Those helpers are now standalone functions in shared-manager/fs-helpers.ts and symlink-helpers.ts that take explicit args. Cleanest path: rebase onto current dev and move syncSkills into its own submodule (for example shared-manager/skills-sync.ts) with a thin delegating method on the orchestrator, mirroring how syncProjectContext is wired.

  2. Isolation gets wiped on launch. skills is already a managed shared item, and ensureInstance runs your syncSkills and then linkSharedDirectories, which removes the instance skills path and re symlinks it to the shared root on every profile launch. So an isolated skills directory never survives, even right after ccs skills <profile> isolate. skills_mode needs to be integrated into the shared linker (skip or rebuild the skills entry when the mode is isolated) rather than added as a parallel manager, so that a single code path owns the instance skills path.

The earlier blockers still stand too: the npx spawn uses shell: true with user supplied args (command injection), and npx skills pulls an unpinned external package at runtime (pin a version, pass --yes). Tests should target the rebased shape and add a regression that runs syncSkills then linkSharedDirectories and asserts the isolated directory survives.

The feature direction is still welcome. Are you still planning to take this forward? If so, a rebase plus the single owner change above is the way in. If not, no worries, just say the word and we can pick it up or close it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:cli-runtime CLI runtime commands, execution flow, and core target behavior enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants