refactor(skills): unify installation in setup, auto-discover from disk

[L1nusB]

Description

Summary

• Removes skill installation from analyze (was a side effect of indexing)
• Makes setup the single owner of skill installation (global only)
• Auto-discovers skill names from the skills/ directory instead of hardcoding them
• Adds migration handling for users with stale project-local skills

Problem

When users run both gitnexus setup and gitnexus analyze, the same skills get installed to two locations:

Location	Installed by
~/.claude/skills/gitnexus-exploring/SKILL.md	setup (global)
<repo>/.claude/skills/gitnexus/gitnexus-exploring/SKILL.md	analyze (project-local)

This triggers Claude Code bug #25209 — both copies appear in the skill list instead of one shadowing the other. Users see every GitNexus skill listed twice.

Beyond the duplication bug, skills are static markdown files that don't depend on the repository index. Installing them on every analyze run is unnecessary work in the wrong place. There were also two separate implementations of the same install logic.

Solution

Single owner: setup installs skills globally to ~/.claude/skills/, ~/.cursor/skills/, and ~/.config/opencode/skill/. analyze no longer touches skills.

Migration: analyze prints a deprecation notice if stale project-local skills exist. setup cleans them up after installing globally.

Auto-discovery: Skill names are discovered from the skills/ source directory at install time instead of being hardcoded. This automatically picks up gitnexus-pr-review (previously missing) and prevents future breakage when skills are added or renamed.

Changes

File	What changed
gitnexus/src/cli/ai-context.ts	Removed installSkills() function and its call; cleaned up unused imports
gitnexus/src/cli/analyze.ts	Added checkStaleProjectSkills() deprecation notice; updated setup tip to mention skills
gitnexus/src/cli/setup.ts	Added discoverSkillNames() replacing hardcoded list; added cleanupProjectLocalSkills() with worktree/submodule support
gitnexus/test/unit/ai-context.test.ts	Acceptance tests: analyze no longer installs skills; regression guards for dynamic context generation
gitnexus/test/unit/setup-skills.test.ts	installSkillsTo tests, setupCommand cleanup tests, discoverSkillNames discovery tests
gitnexus/test/unit/analyze-skills-notice.test.ts	Contract tests for checkStaleProjectSkills export and behavior
README.md	Fixed skill count (4 -> 7), corrected command responsibility descriptions

Design decisions

1. Why not keep both locations? Adds complexity to work around a bug that shouldn't exist in our code. Skills are static config — one canonical location is cleaner.

2. Why doesn't analyze delete stale skills? After the refactor, analyze no longer owns skills. Deleting them would cross the responsibility boundary. It warns; setup cleans up.

3. Why auto-discover instead of hardcode? The hardcoded SKILL_NAMES list was the root cause of gitnexus-pr-review being silently excluded. Discovery from disk means adding a new skill is just dropping a file — no code change required.

4. Worktree/submodule support: cleanupProjectLocalSkills walks upward to find the repo root, handling .git as either a directory (standard) or a file (worktrees/submodules).

Test plan

• analyze no longer creates .claude/skills/gitnexus/ (acceptance tests 1-2)
• analyze prints deprecation notice when stale skills exist (15-17)
• analyze shows notice even on "Already up to date" early return
• setup installs all 7 discovered skills with non-empty SKILL.md (7-9)
• setup removes project-local skills in git repos (11-12)
• setup handles nested dirs, worktrees, non-git dirs, empty dirs (13-14, nested/worktree tests)
• discoverSkillNames discovers flat files, directories, mixed layouts (27-33)
• discoverSkillNames filters to gitnexus-* prefix only (28)
• discoverSkillNames ignores directories without SKILL.md (33)
• discoverSkillNames documents collision behavior for same-name flat+directory entries (34)
• discoverSkillNames propagates errors: missing root (ENOENT), permission failure (EACCES) (35-36)
• SKILL_NAMES lazy-population contract: starts empty, populated after first install (37)
• All existing tests unaffected — 874/874 passing

[vercel]

@L1nusB is attempting to deploy a commit to the NexusCore Team on Vercel.

A member of the Team first needs to authorize it.

[abhigyanpatwari]

🟡 GitNexus Blast Radius: MEDIUM

Metric	Count
Changed symbols	7
Direct dependents (d=1)	2
Indirect (d=2)	0
Transitive (d=3)	0
Flows impacted	3
Total affected	2

Changed: AnalyzeOptions, analyzeCommand, sigintHandler, installSkillsTo, copyDirRecursive, installOpenCodeSkills, setupCommand
Flows hit: SetupCommand → CopyDirRecursive (4 symbols hit), SetupCommand → DirExists (1 symbols hit), SetupCommand → GetMcpEntry (1 symbols hit)

View full blast radius graph →

---

Generated by GitNexus — code intelligence powered by knowledge graphs

[xkonjin]

Quick review pass:

• Main risk area here is input validation, path handling, and malformed payload behavior.
• Good to see test coverage move with the code; I’d still make sure it exercises the unhappy path around input validation, path handling, and malformed payload behavior rather than only the happy path.
• Before merge, I’d smoke-test the behavior touched by marketplace.json, SKILL.md, SKILL.md (+12 more) with malformed input / retry / rollback cases, since that’s where this class of change usually breaks.

[reversTeam]

Good refactoring work, @L1nusB. This PR cleanly separates concerns by moving skill installation out of analyze (which runs frequently) into setup (which runs once), where it belongs.

Key changes reviewed:

• ai-context.ts: Removed installSkills() and its 6-skill hardcoded list. The __dirname setup is also removed since it was only needed for skill file resolution. Clean removal.
• analyze.ts: New checkStaleProjectSkills() that warns without deleting — good migration behavior. Called on both the early-return (already up to date) and full-analyze paths, so users always see the notice.
• setup.ts: discoverSkillNames() dynamically resolves skills from the filesystem instead of a hardcoded array. installSkillsTo() is now exported for testing. cleanupProjectLocalSkills() with findRepoRoot() safely removes stale project-local skills, but only after confirming global skills installed successfully — nice guard.
• README.md: Updated to reflect the new separation (analyze = index, setup = MCP + skills + hooks). Claude Code plugin section is a good addition.
• Tests: Updated ai-context.test.ts to assert skills are NOT installed by analyze. New analyze-skills-notice.test.ts and setup-skills.test.ts with good coverage of the migration path.

One observation: The marketplace.json homepage URL changed from nicosxt/gitnexus to abhigyanpatwari/GitNexus — is this an intentional ownership transfer, or was it included by mistake? Worth double-checking.

LGTM overall — solid refactor with good test coverage and a clean migration path.

[L1nusB]

Merge conflict removal

[github-actions]

CI Report

✅ All checks passed

Pipeline Status

Stage	Status	Details
✅ Typecheck	success	tsc --noEmit
✅ Tests	success	unit tests, 3 platforms
✅ E2E	success	gitnexus-web changes only

Test Results

Tests	Passed	Failed	Skipped	Duration
4877	4831	0	46	183s

✅ All 4831 tests passed

46 test(s) skipped — expand for details

• buildTypeEnv > known limitations (documented skip tests) > Ruby block parameter: users.each { |user| } — closure param inference, different feature
• Swift constructor-inferred type resolution > detects User and Repo classes, both with save methods
• Swift constructor-inferred type resolution > resolves user.save() to Models/User.swift via constructor-inferred type
• Swift constructor-inferred type resolution > resolves repo.save() to Models/Repo.swift via constructor-inferred type
• Swift constructor-inferred type resolution > emits exactly 2 save() CALLS edges (one per receiver type)
• Swift self resolution > detects User and Repo classes, each with a save function
• Swift self resolution > resolves self.save() inside User.process to User.save, not Repo.save
• Swift parent resolution > detects BaseModel and User classes plus Serializable protocol
• Swift parent resolution > emits EXTENDS edge: User → BaseModel
• Swift parent resolution > emits IMPLEMENTS edge: User → Serializable (protocol conformance)
• Swift cross-file User.init() inference > resolves user.save() via User.init(name:) inference
• Swift cross-file User.init() inference > resolves user.greet() via User.init(name:) inference
• Swift return type inference > detects User class and getUser function
• Swift return type inference > detects save function on User (Swift class methods are Function nodes)
• Swift return type inference > resolves user.save() to User#save via return type of getUser() -> User
• Swift return-type inference via function return type > resolves user.save() to User#save via return type of getUser()
• Swift return-type inference via function return type > user.save() does NOT resolve to Repo#save
• Swift return-type inference via function return type > resolves repo.save() to Repo#save via return type of getRepo()
• Swift implicit imports (cross-file visibility) > detects UserService class in Models.swift
• Swift implicit imports (cross-file visibility) > resolves UserService() constructor call across files (no explicit import)
• Swift implicit imports (cross-file visibility) > resolves service.fetchUser() member call across files
• Swift implicit imports (cross-file visibility) > creates IMPORTS edges between files in the same module
• Swift extension deduplication > detects Product class
• Swift extension deduplication > resolves Product() constructor despite extension creating duplicate class node
• Swift extension deduplication > resolves product.save() to Product.swift (primary definition)
• Swift constructor call fallback (no new keyword) > resolves OCRService() as constructor call across files
• Swift constructor call fallback (no new keyword) > resolves ocr.recognize() member call via constructor-inferred type
• Swift export visibility (internal vs private) > resolves PublicService() constructor across files
• Swift export visibility (internal vs private) > resolves internalHelper() across files (internal = module-scoped)
• Swift if let / guard let binding resolution > detects User and Repo classes
• Swift if let / guard let binding resolution > resolves user.save() inside if-let to User#save
• Swift if let / guard let binding resolution > resolves repo.save() inside guard-let to Repo#save
• Swift if let / guard let binding resolution > user.save() in if-let does NOT resolve to Repo#save
• Swift await / try expression unwrapping > resolves user.save() via await fetchUser() return type
• Swift await / try expression unwrapping > resolves repo.save() via try parseRepo() return type
• Swift await / try expression unwrapping > detects fetchUser and parseRepo as functions
• Swift for-in loop element type inference > detects User and Repo classes
• Swift for-in loop element type inference > creates implicit import edges between files
• Swift field-type resolution > detects classes and their properties
• Swift field-type resolution > emits HAS_PROPERTY edges from class to field
• Swift field-type resolution > resolves field-chain call user.address.save() → Address#save
• Swift field-type resolution > emits ACCESSES edges for field reads in chains
• Swift field-type resolution > populates field metadata (visibility, declaredType) on Property nodes
• Swift call-result binding > resolves call-result-bound method call user.save() → User#save
• Swift call-result binding > getUser() is present as a defined function
• Swift call-result binding > emits processUser -> getUser CALLS edge for let-assigned free function call

Code Coverage

Tests

Metric	Coverage	Covered	Base	Delta	Status
Statements	70.88%	13258/18703	70.85%	📈 +0.0	🟢 ██████████████░░░░░░
Branches	60.09%	8763/14581	60.09%	＝ 0.0	🟢 ████████████░░░░░░░░
Functions	75.62%	1173/1551	75.59%	📈 +0.0	🟢 ███████████████░░░░░
Lines	73.1%	12046/16477	73.06%	📈 +0.0	🟢 ██████████████░░░░░░

---

_{📋 View full run · Generated by CI}

[magyargergo]

⚠️ Upcoming Prettier formatting — rebase instructions

PR #563 adds Prettier as the code formatter for the repo. When it merges, the bulk format commit will touch ~350 files (style-only: whitespace, quotes, trailing commas). Your branch will likely conflict.

After #563 merges, rebase your branch:

git fetch origin
git checkout <your-branch>
git rebase origin/main

# Conflicts will be formatting-only — accept your version:
git checkout --theirs .
git add .
git rebase --continue

# Then re-format your branch to match the new style:
npx prettier --write .
git add -A
git commit -m "style: apply prettier formatting"
git push --force-with-lease

New setup step: Run npm install at the repo root (not just in gitnexus/) to get prettier + activate the pre-commit hook. The hook auto-formats staged files on every commit going forward.

[magyargergo]

Please submit a new PR if this is still relevant