mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-04 22:49:41 +02:00
feat(skill): add the agent-skill install layer and harden the packaged skill
Ship `skills/codeman` as an installable Claude Code skill rather than a repo-only reference, and fix six defects found while verifying it live. Install layer: - `codeman skill install [--case <name>]` / `codeman skill uninstall`. Case names resolve through linked-cases.json first, mirroring the server's resolveCasePath(), so a case linked in from outside ~/codeman-cases no longer fails with "Case not found". - applyAgentSkill() / installAgentSkillInto() / removeAgentSkillFrom() in hooks-config.ts. Copies are marker-owned, so an unmarked user-authored skill is never touched, and a symlinked skill dir is refused (this repo's own .claude/skills/codeman is a symlink to the source). - Synced `agentSkillEnabled` setting, default OFF: schemas.ts, ports/config-port.ts, server.ts, session-routes.ts (add-only injection on Claude session create and quick-start), plus the App Settings toggle. Skill content fixes, each reproduced before and after: - Fail-closed `delete_session` replaces `is_self ... || curl -X DELETE`. Shell state does not survive between agent tool calls, and an undefined is_self exited 127, firing the `||` branch and deleting the caller's own session with the one guard bypassed. The request now lives inside the guard, so a lost preamble deletes nothing. - clientId is a fixed literal instead of `agent-$$`. The pid changes per tool call, so the documented resend-identical-request loop stopped being a duplicate and retyped the prompt, submitting the turn twice. - `last-response` is now the documented read path for claude and codex workers. It returns clean transcript text; the terminal scrape it replaces returns a wall of TUI repaint noise. Its transcript flush lags the stop signal, so the recipes poll it rather than reading once. - quick-start examples branch on `.success`. Previously a failed spawn yielded the literal session id "null" and burned the whole readiness budget before reporting jq noise instead of the cause. - Documented that turning `agentSkillEnabled` off sweeps nothing, and corrected the hooks-config comment that claimed a toggle-off sweep exists. Per-case cleanup is `codeman skill uninstall --case <name>`. - Documented that SESSION_BUSY means the 50-session cap on quick-start, and that caseName resolves linked cases, so a generic name can land a worker in a real repo. Tests: test/agent-skill.test.ts covers install, refresh, idempotence, marker ownership and symlink refusal against the real packaged source; test/quick-start.test.ts covers injection behind the setting. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Codeman maintainer
@@ -0,0 +1,122 @@
|
||||
/**
|
||||
* @fileoverview Unit tests for the agent-skill injection helpers in hooks-config.ts
|
||||
* (`applyAgentSkill`, `installAgentSkillInto`, `removeAgentSkillFrom`).
|
||||
*
|
||||
* These run against the REAL packaged source (`skills/codeman/` at the repo root),
|
||||
* so they double as a guard that the skill files exist and are readable: an npm
|
||||
* 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.
|
||||
*/
|
||||
|
||||
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
|
||||
import { mkdtemp, rm, mkdir, writeFile, readFile, symlink, readdir } from 'node:fs/promises';
|
||||
import { existsSync } from 'node:fs';
|
||||
import { join } from 'node:path';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { applyAgentSkill, installAgentSkillInto, removeAgentSkillFrom } from '../src/hooks-config.js';
|
||||
|
||||
const MARKER_PREFIX = '<!-- codeman-managed-agent-skill';
|
||||
|
||||
let casePath: string;
|
||||
const skillDir = () => join(casePath, '.claude', 'skills', 'codeman');
|
||||
|
||||
beforeEach(async () => {
|
||||
casePath = await mkdtemp(join(tmpdir(), 'codeman-agent-skill-'));
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await rm(casePath, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
describe('installAgentSkillInto / applyAgentSkill(enabled)', () => {
|
||||
it('installs SKILL.md (marker appended) and the reference files from the packaged source', async () => {
|
||||
const result = await applyAgentSkill(casePath, true);
|
||||
expect(result).toBe('installed');
|
||||
|
||||
const skillMd = await readFile(join(skillDir(), 'SKILL.md'), 'utf-8');
|
||||
expect(skillMd.startsWith('---\nname: codeman')).toBe(true);
|
||||
expect(skillMd).toContain(MARKER_PREFIX);
|
||||
|
||||
// Reference files ride along byte-for-byte (no marker there).
|
||||
const sourceEndpoints = await readFile(
|
||||
join(process.cwd(), 'skills', 'codeman', 'reference', 'endpoints.md'),
|
||||
'utf-8'
|
||||
);
|
||||
const injectedEndpoints = await readFile(join(skillDir(), 'reference', 'endpoints.md'), 'utf-8');
|
||||
expect(injectedEndpoints).toBe(sourceEndpoints);
|
||||
expect(existsSync(join(skillDir(), 'reference', 'recipes.md'))).toBe(true);
|
||||
});
|
||||
|
||||
it('is idempotent: a second run reports unchanged', async () => {
|
||||
await applyAgentSkill(casePath, true);
|
||||
expect(await applyAgentSkill(casePath, true)).toBe('unchanged');
|
||||
});
|
||||
|
||||
it('refreshes a stale Codeman-managed copy back to the packaged content', async () => {
|
||||
await applyAgentSkill(casePath, true);
|
||||
const original = await readFile(join(skillDir(), 'SKILL.md'), 'utf-8');
|
||||
// Simulate an older injected version: content differs but the marker is intact.
|
||||
await writeFile(join(skillDir(), 'SKILL.md'), `stale content\n${MARKER_PREFIX}: old -->\n`);
|
||||
|
||||
expect(await applyAgentSkill(casePath, true)).toBe('refreshed');
|
||||
expect(await readFile(join(skillDir(), 'SKILL.md'), 'utf-8')).toBe(original);
|
||||
});
|
||||
|
||||
it('never clobbers a user-authored skills/codeman (no marker)', async () => {
|
||||
await mkdir(skillDir(), { recursive: true });
|
||||
await writeFile(join(skillDir(), 'SKILL.md'), '---\nname: codeman\n---\nmy own skill\n');
|
||||
|
||||
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);
|
||||
});
|
||||
|
||||
it('refuses to write through a symlinked skill dir (dogfooding layout)', async () => {
|
||||
await mkdir(join(casePath, '.claude', 'skills'), { recursive: true });
|
||||
await symlink(join(casePath, 'elsewhere'), skillDir());
|
||||
expect(await installAgentSkillInto(skillDir())).toBe('symlink');
|
||||
});
|
||||
|
||||
it('refuses to write through a symlinked skills/ parent', async () => {
|
||||
await mkdir(join(casePath, 'real-skills'), { recursive: true });
|
||||
await mkdir(join(casePath, '.claude'), { recursive: true });
|
||||
await symlink(join(casePath, 'real-skills'), join(casePath, '.claude', 'skills'));
|
||||
expect(await installAgentSkillInto(skillDir())).toBe('symlink');
|
||||
expect(await readdir(join(casePath, 'real-skills'))).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('removeAgentSkillFrom / applyAgentSkill(disabled)', () => {
|
||||
it('removes our copy and prunes the emptied directories', async () => {
|
||||
await applyAgentSkill(casePath, true);
|
||||
expect(await applyAgentSkill(casePath, false)).toBe('removed');
|
||||
expect(existsSync(skillDir())).toBe(false);
|
||||
expect(existsSync(join(casePath, '.claude', 'skills'))).toBe(false);
|
||||
// `.claude` itself is not ours to prune.
|
||||
expect(existsSync(join(casePath, '.claude'))).toBe(true);
|
||||
});
|
||||
|
||||
it('reports absent when there is nothing to remove', async () => {
|
||||
expect(await applyAgentSkill(casePath, false)).toBe('absent');
|
||||
});
|
||||
|
||||
it('leaves a user-authored copy untouched', async () => {
|
||||
await mkdir(skillDir(), { recursive: true });
|
||||
await writeFile(join(skillDir(), 'SKILL.md'), 'my own skill\n');
|
||||
expect(await applyAgentSkill(casePath, false)).toBe('foreign');
|
||||
expect(existsSync(join(skillDir(), 'SKILL.md'))).toBe(true);
|
||||
});
|
||||
|
||||
it("preserves a user's extra files in the directory (no rm -rf)", async () => {
|
||||
await applyAgentSkill(casePath, true);
|
||||
await writeFile(join(skillDir(), 'reference', 'my-notes.md'), 'mine\n');
|
||||
|
||||
expect(await applyAgentSkill(casePath, false)).toBe('removed');
|
||||
expect(existsSync(join(skillDir(), 'SKILL.md'))).toBe(false);
|
||||
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');
|
||||
});
|
||||
});
|
||||
@@ -86,6 +86,7 @@ export function createMockRouteContext(options?: { sessionId?: string }) {
|
||||
getModelConfig: vi.fn(async () => null),
|
||||
getClaudeModeConfig: vi.fn(async () => ({})),
|
||||
getTerminalHistoryConfig: vi.fn(async () => resolveTerminalHistoryConfig({})),
|
||||
getAgentSkillEnabled: vi.fn(async () => false),
|
||||
getDefaultClaudeMdPath: vi.fn(async () => undefined),
|
||||
getLightState: vi.fn(() => ({ sessions: [], status: 'ok' })),
|
||||
getLightSessionsState: vi.fn(() => {
|
||||
|
||||
@@ -332,3 +332,83 @@ describe('Case Management', () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('Agent skill injection (agentSkillEnabled)', () => {
|
||||
let server: WebServer;
|
||||
let baseUrl: string;
|
||||
const createdCases: string[] = [];
|
||||
|
||||
beforeAll(async () => {
|
||||
server = await createTestServer(TEST_PORT + 4); // 3103
|
||||
await server.start();
|
||||
baseUrl = `http://localhost:${TEST_PORT + 4}`;
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await server.stop();
|
||||
for (const caseName of createdCases) {
|
||||
const casePath = join(CASES_DIR, caseName);
|
||||
if (existsSync(casePath)) {
|
||||
rmSync(casePath, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it('does not inject by default, accepts the setting via PUT, then injects on quick-start', async () => {
|
||||
// 1. Default OFF: a claude quick-start creates the case without the skill.
|
||||
const offCase = 'test-skill-off-' + Date.now();
|
||||
createdCases.push(offCase);
|
||||
const offResponse = await fetch(`${baseUrl}/api/quick-start`, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ caseName: offCase }),
|
||||
});
|
||||
const offData = await offResponse.json();
|
||||
expect(offData.success).toBe(true);
|
||||
expect(existsSync(join(CASES_DIR, offCase, '.claude', 'skills', 'codeman'))).toBe(false);
|
||||
|
||||
// 2. The `.strict()` settings schema accepts the new synced key.
|
||||
const putResponse = await fetch(`${baseUrl}/api/settings`, {
|
||||
method: 'PUT',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ agentSkillEnabled: true }),
|
||||
});
|
||||
const putData = await putResponse.json();
|
||||
expect(putData.success).toBe(true);
|
||||
|
||||
// 3. The server's settings read is cached ~2s; outwait it so the create sees the toggle.
|
||||
await new Promise((resolve) => setTimeout(resolve, 2100));
|
||||
|
||||
// 4. Quick-start now injects the marker-carrying skill into the new case.
|
||||
const onCase = 'test-skill-on-' + Date.now();
|
||||
createdCases.push(onCase);
|
||||
const onResponse = await fetch(`${baseUrl}/api/quick-start`, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ caseName: onCase }),
|
||||
});
|
||||
const onData = await onResponse.json();
|
||||
expect(onData.success).toBe(true);
|
||||
|
||||
const skillDir = join(CASES_DIR, onCase, '.claude', 'skills', 'codeman');
|
||||
const { readFileSync } = await import('node:fs');
|
||||
const skillMd = readFileSync(join(skillDir, 'SKILL.md'), 'utf-8');
|
||||
expect(skillMd.startsWith('---\nname: codeman')).toBe(true);
|
||||
expect(skillMd).toContain('<!-- codeman-managed-agent-skill');
|
||||
expect(existsSync(join(skillDir, 'reference', 'endpoints.md'))).toBe(true);
|
||||
expect(existsSync(join(skillDir, 'reference', 'recipes.md'))).toBe(true);
|
||||
}, 30000);
|
||||
|
||||
it('does not inject for shell-mode quick-start even when enabled', async () => {
|
||||
const shellCase = 'test-skill-shell-' + Date.now();
|
||||
createdCases.push(shellCase);
|
||||
const response = await fetch(`${baseUrl}/api/quick-start`, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ caseName: shellCase, mode: 'shell' }),
|
||||
});
|
||||
const data = await response.json();
|
||||
expect(data.success).toBe(true);
|
||||
expect(existsSync(join(CASES_DIR, shellCase, '.claude', 'skills', 'codeman'))).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user