mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-09-22 00:31:17 +00:00
fix(cli): address PR #742 review — gate community skills, drop dangling refs, add tests
Bot review (#742) flagged three issues with the original commit: 1. `--index-only --skills` still wrote community-derived skill files to `.claude/skills/generated/`. The `--skills` branch in analyze.ts was not gated by `skipAll`, so the "skip all file injection" contract was violated. Gate `generateSkillFiles()` with `!skipAll` so `--index-only` truly wins over `--skills`. 2. `--skip-skills` without `--skip-agents-md` produced AGENTS.md / CLAUDE.md that still referenced `.claude/skills/gitnexus/*/SKILL.md` files that were never installed — every agent load incurred 6 failed reads. Pass `skipSkills` through to `generateGitNexusContent()` and omit the standard-skill rows (and the entire `## CLI` heading when the table is empty). Community skills, when present via `--skills`, are unaffected. 3. No filesystem tests for `skipSkills` / `indexOnly`. Add three regression guards to `test/unit/ai-context.test.ts`: - `.claude/skills/gitnexus/` is NOT created when skipSkills=true - Nothing is written when both skipAgentsMd and skipSkills are true (the resolved-flag state from --index-only) - AGENTS.md/CLAUDE.md routing table omits standard skill references when skipSkills=true, but preserves the load-bearing imperative sections (Always Do / Never Do / Resources)
This commit is contained in:
parent
666f87f8c7
commit
bf6db0e728
3 changed files with 124 additions and 8 deletions
|
|
@ -95,6 +95,7 @@ function generateGitNexusContent(
|
|||
generatedSkills?: GeneratedSkillInfo[],
|
||||
groupNames?: string[],
|
||||
noStats?: boolean,
|
||||
skipSkills?: boolean,
|
||||
): string {
|
||||
const generatedRows =
|
||||
generatedSkills && generatedSkills.length > 0
|
||||
|
|
@ -106,14 +107,26 @@ function generateGitNexusContent(
|
|||
.join('\n')
|
||||
: '';
|
||||
|
||||
const skillsTable = `| Task | Read this skill file |
|
||||
|------|---------------------|
|
||||
| Understand architecture / "How does X work?" | \`.claude/skills/gitnexus/gitnexus-exploring/SKILL.md\` |
|
||||
// Standard skill rows reference files installed by installSkills(). When
|
||||
// --skip-skills suppresses that install, these rows must be omitted — else
|
||||
// AGENTS.md/CLAUDE.md would direct agents to read files that don't exist.
|
||||
// Community skills (generatedRows) live in .claude/skills/generated/ and
|
||||
// are independent of --skip-skills, so they remain when present.
|
||||
const standardSkillsRows = skipSkills
|
||||
? ''
|
||||
: `| Understand architecture / "How does X work?" | \`.claude/skills/gitnexus/gitnexus-exploring/SKILL.md\` |
|
||||
| Blast radius / "What breaks if I change X?" | \`.claude/skills/gitnexus/gitnexus-impact-analysis/SKILL.md\` |
|
||||
| Trace bugs / "Why is X failing?" | \`.claude/skills/gitnexus/gitnexus-debugging/SKILL.md\` |
|
||||
| Rename / extract / split / refactor | \`.claude/skills/gitnexus/gitnexus-refactoring/SKILL.md\` |
|
||||
| Tools, resources, schema reference | \`.claude/skills/gitnexus/gitnexus-guide/SKILL.md\` |
|
||||
| Index, status, clean, wiki CLI commands | \`.claude/skills/gitnexus/gitnexus-cli/SKILL.md\` |${generatedRows ? '\n' + generatedRows : ''}`;
|
||||
| Index, status, clean, wiki CLI commands | \`.claude/skills/gitnexus/gitnexus-cli/SKILL.md\` |`;
|
||||
|
||||
const tableBody = [standardSkillsRows, generatedRows].filter(Boolean).join('\n');
|
||||
const skillsTable = tableBody
|
||||
? `| Task | Read this skill file |
|
||||
|------|---------------------|
|
||||
${tableBody}`
|
||||
: '';
|
||||
|
||||
return `${GITNEXUS_START_MARKER}
|
||||
# GitNexus — Code Intelligence
|
||||
|
|
@ -154,11 +167,15 @@ This repository is listed under GitNexus **group(s): ${groupNames.join(', ')}**
|
|||
|
||||
`
|
||||
: ''
|
||||
}## CLI
|
||||
}${
|
||||
skillsTable
|
||||
? `## CLI
|
||||
|
||||
${skillsTable}
|
||||
|
||||
${GITNEXUS_END_MARKER}`;
|
||||
`
|
||||
: ''
|
||||
}${GITNEXUS_END_MARKER}`;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -320,6 +337,7 @@ export async function generateAIContextFiles(
|
|||
generatedSkills,
|
||||
groupNames,
|
||||
options?.noStats,
|
||||
options?.skipSkills,
|
||||
);
|
||||
const createdFiles: string[] = [];
|
||||
|
||||
|
|
|
|||
|
|
@ -464,8 +464,11 @@ export const analyzeCommand = async (inputPath?: string, options?: AnalyzeOption
|
|||
// a healthy index.
|
||||
await assertAnalysisFinalized(repoPath);
|
||||
|
||||
// Skill generation (CLI-only, uses pipeline result from analysis)
|
||||
if (options?.skills && result.pipelineResult) {
|
||||
// Skill generation (CLI-only, uses pipeline result from analysis).
|
||||
// Gated by !skipAll so `--index-only --skills` truly skips ALL file
|
||||
// injection — otherwise `generateSkillFiles()` would still write
|
||||
// community-derived skill files to .claude/skills/generated/.
|
||||
if (options?.skills && result.pipelineResult && !skipAll) {
|
||||
updateBar(99, 'Generating skill files...');
|
||||
try {
|
||||
const { generateSkillFiles } = await import('./skill-gen.js');
|
||||
|
|
|
|||
|
|
@ -137,6 +137,101 @@ describe('generateAIContextFiles', () => {
|
|||
}
|
||||
});
|
||||
|
||||
it('does not create .claude/skills/gitnexus/ when skipSkills is true (#742)', async () => {
|
||||
// Regression guard for #742. The --skip-skills flag must prevent
|
||||
// installSkills() from writing the 6 standard skill dirs into the
|
||||
// analyzed repo. Per-test tmpdir so we start from a known-clean
|
||||
// slate — the shared tmpDir from beforeAll may already contain
|
||||
// .claude/skills/gitnexus/ from an earlier test.
|
||||
const skipDir = await fs.mkdtemp(path.join(os.tmpdir(), 'gn-ai-ctx-skip-skills-'));
|
||||
const skipStorage = path.join(skipDir, '.gitnexus');
|
||||
await fs.mkdir(skipStorage, { recursive: true });
|
||||
try {
|
||||
const stats = { nodes: 50, edges: 100, processes: 5 };
|
||||
const result = await generateAIContextFiles(
|
||||
skipDir,
|
||||
skipStorage,
|
||||
'TestProject',
|
||||
stats,
|
||||
undefined,
|
||||
{ skipSkills: true },
|
||||
);
|
||||
|
||||
expect(result.files).toContain('.claude/skills/gitnexus/ (skipped via --skip-skills)');
|
||||
await expect(
|
||||
fs.access(path.join(skipDir, '.claude', 'skills', 'gitnexus')),
|
||||
).rejects.toThrow();
|
||||
} finally {
|
||||
await fs.rm(skipDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it('writes nothing when both skipAgentsMd and skipSkills are true (--index-only, #742)', async () => {
|
||||
// Regression guard for #742. analyzeCommand() resolves --index-only
|
||||
// into BOTH skipAgentsMd=true and skipSkills=true. This test pins
|
||||
// the resolved-flag combination so a future regression that drops
|
||||
// either guard fails here. Per-test tmpdir for the same reason as
|
||||
// the skipSkills test above.
|
||||
const idxDir = await fs.mkdtemp(path.join(os.tmpdir(), 'gn-ai-ctx-index-only-'));
|
||||
const idxStorage = path.join(idxDir, '.gitnexus');
|
||||
await fs.mkdir(idxStorage, { recursive: true });
|
||||
try {
|
||||
const stats = { nodes: 50, edges: 100, processes: 5 };
|
||||
const result = await generateAIContextFiles(
|
||||
idxDir,
|
||||
idxStorage,
|
||||
'TestProject',
|
||||
stats,
|
||||
undefined,
|
||||
{ skipAgentsMd: true, skipSkills: true },
|
||||
);
|
||||
|
||||
expect(result.files).toContain('AGENTS.md (skipped via --skip-agents-md)');
|
||||
expect(result.files).toContain('CLAUDE.md (skipped via --skip-agents-md)');
|
||||
expect(result.files).toContain('.claude/skills/gitnexus/ (skipped via --skip-skills)');
|
||||
|
||||
await expect(fs.access(path.join(idxDir, 'AGENTS.md'))).rejects.toThrow();
|
||||
await expect(fs.access(path.join(idxDir, 'CLAUDE.md'))).rejects.toThrow();
|
||||
await expect(fs.access(path.join(idxDir, '.claude', 'skills', 'gitnexus'))).rejects.toThrow();
|
||||
} finally {
|
||||
await fs.rm(idxDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it('omits standard skill references from AGENTS.md/CLAUDE.md when skipSkills is true (#742)', async () => {
|
||||
// The skills routing table in AGENTS.md/CLAUDE.md points agents at
|
||||
// .claude/skills/gitnexus/*/SKILL.md files installed by installSkills().
|
||||
// When --skip-skills suppresses that install but AGENTS.md/CLAUDE.md
|
||||
// are still written, the routing table must NOT name files that don't
|
||||
// exist — otherwise every agent load incurs 6 failed reads and the
|
||||
// routing instructions are worthless. Per-test tmpdir so the assertions
|
||||
// are not contaminated by a CLAUDE.md from an earlier test.
|
||||
const noStdDir = await fs.mkdtemp(path.join(os.tmpdir(), 'gn-ai-ctx-no-std-skills-'));
|
||||
const noStdStorage = path.join(noStdDir, '.gitnexus');
|
||||
await fs.mkdir(noStdStorage, { recursive: true });
|
||||
try {
|
||||
const stats = { nodes: 50, edges: 100, processes: 5 };
|
||||
await generateAIContextFiles(noStdDir, noStdStorage, 'TestProject', stats, undefined, {
|
||||
skipSkills: true,
|
||||
});
|
||||
|
||||
const content = await fs.readFile(path.join(noStdDir, 'CLAUDE.md'), 'utf-8');
|
||||
expect(content).not.toContain('gitnexus-exploring/SKILL.md');
|
||||
expect(content).not.toContain('gitnexus-impact-analysis/SKILL.md');
|
||||
expect(content).not.toContain('gitnexus-debugging/SKILL.md');
|
||||
expect(content).not.toContain('gitnexus-refactoring/SKILL.md');
|
||||
expect(content).not.toContain('gitnexus-guide/SKILL.md');
|
||||
expect(content).not.toContain('gitnexus-cli/SKILL.md');
|
||||
// The load-bearing imperative sections must still ship — only the
|
||||
// routing rows are conditional.
|
||||
expect(content).toContain('## Always Do');
|
||||
expect(content).toContain('## Never Do');
|
||||
expect(content).toContain('gitnexus://repo/TestProject/context');
|
||||
} finally {
|
||||
await fs.rm(noStdDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it('preserves manual AGENTS.md and CLAUDE.md edits when skipAgentsMd is enabled', async () => {
|
||||
const stats = { nodes: 42, edges: 84, processes: 3 };
|
||||
const agentsPath = path.join(tmpDir, 'AGENTS.md');
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue