From ba9d311cb778cabff52eded69e70e0c834de7ddd Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Sun, 8 Nov 2026 07:35:03 +0100 Subject: [PATCH] feat(agent-skill): make the injected skill copy ignore itself in git Every Codeman-managed copy of the agent skill (`.claude/skills/codeman/`, `.agents/skills/codeman/`, and the user-level copy) now carries a `.gitignore` of `*`, written once with O_EXCL before any skill file, the way the prompt uploads folder does. A user's repository no longer lists the copy as untracked, and Codeman never touches the repository's own ignore files. A file already at that name is never overwritten, a copy injected before this gains the file on its next install (reported as refreshed), and uninstall removes it only when it is the last thing left and holds exactly `*`, so a user's extra files in the folder stay ignored. Claude Code 2.1.319 runs `git check-ignore` only on directories at or above `.claude/skills`, never on a skill folder, so the skill still loads. Co-Authored-By: Claude Opus 5.5 --- src/hooks-config.ts | 69 +++++++++++++++++- test/agent-skill-targets.test.ts | 2 + test/agent-skill.test.ts | 117 ++++++++++++++++++++++++++++++- 3 files changed, 185 insertions(+), 3 deletions(-) diff --git a/src/hooks-config.ts b/src/hooks-config.ts index e1eef5a3..26fc6986 100644 --- a/src/hooks-config.ts +++ b/src/hooks-config.ts @@ -1358,6 +1358,62 @@ async function isSymlink(path: string): Promise { } } +/** + * The ignore file every Codeman-managed skill copy carries, and what it holds: `*` + * matches everything in the copy's own folder (the ignore file included), so a user's + * repository never lists `.claude/skills/codeman/` (or `.agents/skills/codeman/`) as + * untracked, without Codeman touching the repository's own `.gitignore` or + * `info/exclude`. Same idea as the prompt-uploads folder (`ensureUploadDir`). + * + * Deliberately NOT part of `readAgentSkillSource()`: it is written once with O_EXCL and + * never compared or rewritten, so a file the user put there (or edited) is theirs. + * + * Measured against Claude Code 2.1.319, whose skill discovery runs `git check-ignore` + * only on directories at or above `.claude/skills` (the nested-skills walk), never on a + * skill folder or its SKILL.md: a `*` inside `codeman/` ignores the folder's CONTENTS, + * not the folder, so the skill still loads. Codex 0.162 does no gitignore check on skills. + */ +const AGENT_SKILL_GITIGNORE = '.gitignore'; +const AGENT_SKILL_GITIGNORE_CONTENT = '*\n'; + +/** + * Write the copy's `.gitignore` if none is there. Returns whether it wrote one. + * 'wx' (O_CREAT|O_EXCL) never replaces an existing file and refuses a symlink at the + * name. A create that succeeded before the write failed (ENOSPC) would leave an empty + * file that reads as the user's next time, so it is taken back; that cleanup's own + * failure must not replace the cause. + */ +async function ensureAgentSkillGitignore(skillDir: string): Promise { + const ignoreFile = join(skillDir, AGENT_SKILL_GITIGNORE); + try { + await writeFile(ignoreFile, AGENT_SKILL_GITIGNORE_CONTENT, { flag: 'wx' }); + return true; + } catch (err) { + if ((err as NodeJS.ErrnoException).code === 'EEXIST') return false; + await unlink(ignoreFile).catch(() => {}); + throw err; + } +} + +/** + * Remove the copy's `.gitignore` only when it is the LAST thing in the folder and holds + * exactly what Codeman writes. Anything else in the folder (a user's own notes) keeps + * the ignore file, so those files stay out of `git status` as they were, and a + * `.gitignore` with other content is the user's. + */ +async function removeAgentSkillGitignoreIfLast(skillDir: string): Promise { + const ignoreFile = join(skillDir, AGENT_SKILL_GITIGNORE); + try { + const entries = await readdir(skillDir); + if (entries.length !== 1 || entries[0] !== AGENT_SKILL_GITIGNORE) return; + if (!(await lstat(ignoreFile)).isFile()) return; + if ((await readFile(ignoreFile, 'utf-8')) !== AGENT_SKILL_GITIGNORE_CONTENT) return; + await unlink(ignoreFile); + } catch { + // best-effort: a folder that stays behind holds only an ignore file + } +} + /** What an install/remove actually did, so callers (CLI, logs) can say so. */ export type AgentSkillApplyResult = | 'installed' // fresh copy written @@ -1383,6 +1439,12 @@ export type AgentSkillApplyResult = * Idempotent and cheap: unchanged files are not rewritten, so calling on every * session create causes no mtime churn. * + * The copy ignores itself in git: a `.gitignore` of `*` is written FIRST, before any + * skill file, so a `git status` that runs mid-install never lists the folder either + * (`ensureAgentSkillGitignore`). It is written once and never rewritten; a copy made + * before this existed gains it on its next install or refresh, which then reports + * 'refreshed'. + * * Serialized on the skill dir through the same lock the settings writers use: two * sessions created at once in one repo both inject this skill, and interleaving their * ownership read with the other's write reports a bogus result (an 'unchanged' for a @@ -1402,7 +1464,8 @@ export async function installAgentSkillInto(skillDir: string): Promise {}); } await rmdir(join(skillDir, 'reference')).catch(() => {}); // fails when non-empty, fine + await removeAgentSkillGitignoreIfLast(skillDir); await rmdir(skillDir).catch(() => {}); await rmdir(dirname(skillDir)).catch(() => {}); // prune `.claude/skills` (or the CLI's own dir) if now empty return 'removed'; diff --git a/test/agent-skill-targets.test.ts b/test/agent-skill-targets.test.ts index 49d414c8..d0771313 100644 --- a/test/agent-skill-targets.test.ts +++ b/test/agent-skill-targets.test.ts @@ -81,6 +81,8 @@ describe('applyAgentSkill with a directory', () => { expect(skillMd.startsWith('---\nname: codeman')).toBe(true); expect(skillMd).toContain(MARKER_PREFIX); expect(existsSync(join(skillDir, 'reference', 'endpoints.md'))).toBe(true); + // It ignores itself in git there too, like the Claude Code copy. + expect(await readFile(join(skillDir, '.gitignore'), 'utf-8')).toBe('*\n'); expect(existsSync(join(casePath, '.claude'))).toBe(false); expect(await applyAgentSkill(casePath, true, AGENTS_DIR)).toBe('unchanged'); }); diff --git a/test/agent-skill.test.ts b/test/agent-skill.test.ts index f3c9340f..a5d01b56 100644 --- a/test/agent-skill.test.ts +++ b/test/agent-skill.test.ts @@ -7,12 +7,14 @@ * publish without them would be caught here before the `files` entry silently * ignores the missing directory. * - * Pure filesystem tests in a per-test temp dir. Port: N/A. + * Filesystem tests in a per-test temp dir; the self-ignoring block also runs real + * `git` there (no global or system config). Port: N/A. */ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { mkdtemp, rm, mkdir, writeFile, readFile, symlink, readdir, stat, utimes } from 'node:fs/promises'; import { existsSync } from 'node:fs'; +import { execFileSync } from 'node:child_process'; import { join } from 'node:path'; import { tmpdir, homedir } from 'node:os'; import { @@ -92,6 +94,8 @@ describe('installAgentSkillInto / applyAgentSkill(enabled)', () => { expect(await applyAgentSkill(casePath, true)).toBe('foreign'); expect(await readFile(join(skillDir(), 'SKILL.md'), 'utf-8')).toContain('my own skill'); expect(existsSync(join(skillDir(), 'reference'))).toBe(false); + // Not even the ignore file: a foreign copy is the user's to track or not. + expect(existsSync(join(skillDir(), '.gitignore'))).toBe(false); }); it('refuses to write through a symlinked skill dir (dogfooding layout)', async () => { @@ -139,6 +143,117 @@ describe('removeAgentSkillFrom / applyAgentSkill(disabled)', () => { expect(existsSync(join(skillDir(), 'reference', 'endpoints.md'))).toBe(false); // The user's file and the directories holding it survive. expect(await readFile(join(skillDir(), 'reference', 'my-notes.md'), 'utf-8')).toBe('mine\n'); + // And so does the ignore file, so their notes stay out of `git status` as before. + expect(await readFile(join(skillDir(), '.gitignore'), 'utf-8')).toBe('*\n'); + }); +}); + +describe('the injected copy ignores itself in git', () => { + const ENV = { + ...process.env, + LC_ALL: 'C', + GIT_AUTHOR_NAME: 'T', + GIT_AUTHOR_EMAIL: 't@example.com', + GIT_COMMITTER_NAME: 'T', + GIT_COMMITTER_EMAIL: 't@example.com', + GIT_CONFIG_GLOBAL: '/dev/null', + GIT_CONFIG_NOSYSTEM: '1', + }; + const git = (...args: string[]) => + execFileSync('git', args, { cwd: casePath, env: ENV, encoding: 'utf-8', stdio: ['ignore', 'pipe', 'pipe'] }); + /** Exit status of `git check-ignore -q -- `: 0 ignored, 1 not ignored. */ + const checkIgnore = (path: string): number => { + try { + git('check-ignore', '-q', '--', path); + return 0; + } catch (err) { + return (err as { status: number }).status; + } + }; + /** A repository with one commit and a tracked `.gitignore` of its own. */ + const initRepo = async () => { + git('init', '-q', '-b', 'main'); + await writeFile(join(casePath, 'README.md'), 'hello\n'); + await writeFile(join(casePath, '.gitignore'), 'node_modules/\n'); + git('add', 'README.md', '.gitignore'); + git('-c', 'commit.gpgsign=false', 'commit', '-q', '-m', 'init'); + }; + + it('writes a .gitignore of `*` into the copy on install', async () => { + expect(await applyAgentSkill(casePath, true)).toBe('installed'); + expect(await readFile(join(skillDir(), '.gitignore'), 'utf-8')).toBe('*\n'); + // Never part of the skill content: a second run has nothing to do. + expect(await applyAgentSkill(casePath, true)).toBe('unchanged'); + }); + + it('keeps `git status` of the user repository clean, and leaves its own ignore file alone', async () => { + await initRepo(); + await applyAgentSkill(casePath, true); + await applyAgentSkill(casePath, true, '.agents/skills'); + + expect(git('status', '--porcelain', '--untracked-files=all')).toBe(''); + expect(await readFile(join(casePath, '.gitignore'), 'utf-8')).toBe('node_modules/\n'); + // Nor the repository's local exclude file (git's template may or may not create one). + const exclude = join(casePath, '.git', 'info', 'exclude'); + if (existsSync(exclude)) expect(await readFile(exclude, 'utf-8')).not.toContain('codeman'); + }); + + it('ignores the copy contents, never the folder above it (Claude Code checks only those)', async () => { + // Claude Code 2.1.319 skips a nested skills folder when `git check-ignore` says its + // project directory is ignored; it never checks a skill folder or its SKILL.md. + await initRepo(); + await applyAgentSkill(casePath, true); + expect(checkIgnore('.claude/skills/codeman/SKILL.md')).toBe(0); + expect(checkIgnore('.claude/skills/codeman')).toBe(1); + expect(checkIgnore('.claude/skills')).toBe(1); + expect(checkIgnore('.claude')).toBe(1); + }); + + it('never overwrites a .gitignore that is already in the folder', async () => { + await applyAgentSkill(casePath, true); + await writeFile(join(skillDir(), '.gitignore'), '# mine\nSKILL.md\n'); + // A stale copy: the refresh rewrites the skill but not the ignore file. + await writeFile(join(skillDir(), 'SKILL.md'), `stale\n${MARKER_PREFIX}: old -->\n`); + expect(await applyAgentSkill(casePath, true)).toBe('refreshed'); + expect(await readFile(join(skillDir(), '.gitignore'), 'utf-8')).toBe('# mine\nSKILL.md\n'); + }); + + it('a copy injected before the ignore file existed gains it on the next install', async () => { + await applyAgentSkill(casePath, true); + await rm(join(skillDir(), '.gitignore')); + // Every skill file is current, so only the ignore file is new: still a refresh. + expect(await applyAgentSkill(casePath, true)).toBe('refreshed'); + expect(await readFile(join(skillDir(), '.gitignore'), 'utf-8')).toBe('*\n'); + }); + + it('does not write through a symlink planted at the ignore file name', async () => { + await mkdir(skillDir(), { recursive: true }); + const outside = join(casePath, 'outside.txt'); + await writeFile(outside, 'untouched\n'); + await symlink(outside, join(skillDir(), '.gitignore')); + expect(await applyAgentSkill(casePath, true)).toBe('installed'); + expect(await readFile(outside, 'utf-8')).toBe('untouched\n'); + }); + + it('uninstall removes the ignore file with the copy, so no folder is left behind', async () => { + await initRepo(); + await applyAgentSkill(casePath, true); + expect(await applyAgentSkill(casePath, false)).toBe('removed'); + expect(existsSync(skillDir())).toBe(false); + expect(existsSync(join(casePath, '.claude', 'skills'))).toBe(false); + expect(git('status', '--porcelain', '--untracked-files=all')).toBe(''); + }); + + it("uninstall keeps an ignore file that is not Codeman's", async () => { + await applyAgentSkill(casePath, true); + await writeFile(join(skillDir(), '.gitignore'), 'SKILL.md\n'); + expect(await applyAgentSkill(casePath, false)).toBe('removed'); + expect(await readFile(join(skillDir(), '.gitignore'), 'utf-8')).toBe('SKILL.md\n'); + }); + + it('the packaged source carries no ignore file (the plugin mirror pins its file set)', () => { + expect(existsSync(join(process.cwd(), 'skills', 'codeman', '.gitignore'))).toBe(false); + expect(existsSync(join(process.cwd(), 'plugins', 'codeman', 'skills', 'codeman', '.gitignore'))).toBe(false); }); });