mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-11-14 02:29:33 +01:00
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 <noreply@anthropic.com>
This commit is contained in:
1 parent
e9b2ecd552
commit
ba9d311cb7
3 files changed
+185
-3
No files matched your search
+67
-2
@@ -1358,6 +1358,62 @@ async function isSymlink(path: string): Promise<boolean> {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* 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<boolean> {
|
||||
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<void> {
|
||||
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<AgentSkil
|
||||
if (existing !== null && !existing.includes(AGENT_SKILL_MARKER_PREFIX)) return 'foreign';
|
||||
|
||||
const files = await readAgentSkillSource();
|
||||
let changed = false;
|
||||
await mkdir(skillDir, { recursive: true });
|
||||
let changed = await ensureAgentSkillGitignore(skillDir);
|
||||
for (const file of files) {
|
||||
const target = join(skillDir, file.relPath);
|
||||
let current: string | null = null;
|
||||
@@ -1544,7 +1607,8 @@ export async function refreshUserAgentSkill(): Promise<AgentSkillApplyResult | '
|
||||
* Remove a Codeman-managed skill copy from `skillDir`. Same ownership and symlink
|
||||
* refusals as the install path. Deletes only files the packaged source would have
|
||||
* written (never `rm -rf`, so a user's extra files in the directory survive), then
|
||||
* prunes the directories bottom-up if they emptied.
|
||||
* the copy's own `.gitignore` when nothing else is left beside it, then prunes the
|
||||
* directories bottom-up if they emptied.
|
||||
*
|
||||
* Shares the install path's per-dir lock so an uninstall can't run between an install's
|
||||
* ownership read and its writes, which would leave half the skill back on disk.
|
||||
@@ -1568,6 +1632,7 @@ export async function removeAgentSkillFrom(skillDir: string): Promise<AgentSkill
|
||||
await unlink(join(skillDir, file.relPath)).catch(() => {});
|
||||
}
|
||||
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';
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
|
||||
+116
-1
@@ -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 -- <path>`: 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);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
Reference in new issue
Block a user