test: cover the skill CLI, the injection call site, and endpoints.md drift

Three gaps found while auditing the agent skill.

`codeman skill install` / `uninstall` had no tests at all, including the linked-case
resolution that shipped in 1.14.2 with nothing guarding it. Covered now: global target
resolution, `--case` resolving through linked-cases.json, `--case` falling back to the
cases dir for an unlinked name, a missing or malformed registry degrading to the
fallback instead of throwing, and a nonexistent case being rejected. `resolveSkillTarget`
called `process.exit(1)` for a missing case, which would have killed the test runner, so
the pure resolution is split out and exported; CLI behavior is unchanged.

The `POST /api/sessions` injection call site was never exercised, because the shared
route mock hardcoded the gate off. The mock's gate is overridable per test now (default
still off, since other tests rely on that), and there is coverage that the path injects
when the setting is on, does not when it is off, and is claude-mode gated.

Nothing guarded skills/codeman/reference/endpoints.md against drifting from the routes
it documents, which is how it drifted in the first place. A static guard parses the
endpoints out of the markdown and asserts each is really registered, tolerating the
/api/v1 alias and path params.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

src/cli.ts

@@ -136,7 +136,7 @@ const LINKED_CASES_FILE = dataPath('linked-cases.json');
  * Sync and tolerant on purpose: a missing or malformed registry means "no linked
  * cases", never a crash.
  */
-function resolveCliCasePath(name: string): string {
+export function resolveCliCasePath(name: string): string {
   try {
     const linked = JSON.parse(readFileSync(LINKED_CASES_FILE, 'utf-8')) as Record<string, string>;
     const target = linked?.[name];
@@ -154,17 +154,30 @@ function resolveCliCasePath(name: string): string {
  * `resolveCliCasePath()` above. The web server's automatic per-case injection
  * (`agentSkillEnabled`) covers multi-user spaces; this CLI is a local operator tool
  * and stays single-user.
+ *
+ * A missing case is REPORTED, not exited on: the exit lives in the wrapper below so
+ * this resolution (including the linked-cases lookup, which shipped unguarded) can be
+ * unit-tested without `process.exit(1)` taking the test runner down with it.
  */
-function resolveSkillTarget(options: { case?: string }): string {
+export function resolveSkillTargetPath(options: {
+  case?: string;
+}): { target: string; missingCase?: undefined } | { target?: undefined; missingCase: string } {
   if (options.case) {
     const casePath = resolveCliCasePath(options.case);
-    if (!existsSync(casePath)) {
-      console.error(chalk.red(`✗ Case not found: ${casePath}`));
-      process.exit(1);
-    }
-    return join(casePath, '.claude', 'skills', 'codeman');
+    if (!existsSync(casePath)) return { missingCase: casePath };
+    return { target: join(casePath, '.claude', 'skills', 'codeman') };
   }
-  return join(homedir(), '.claude', 'skills', 'codeman');
+  return { target: join(homedir(), '.claude', 'skills', 'codeman') };
+}
+
+/** Exit-owning wrapper around `resolveSkillTargetPath()` for the two commands below. */
+function resolveSkillTarget(options: { case?: string }): string {
+  const resolved = resolveSkillTargetPath(options);
+  if (resolved.missingCase !== undefined) {
+    console.error(chalk.red(`✗ Case not found: ${resolved.missingCase}`));
+    process.exit(1);
+  }
+  return resolved.target;
 }
 
 /** Print an AgentSkillApplyResult for humans; exit non-zero when nothing was done. */

test/agent-skill-endpoints-doc.test.ts

@@ -0,0 +1,88 @@
+/**
+ * @fileoverview Static guard: every endpoint the packaged agent skill documents
+ * still exists in the routes it is documenting.
+ *
+ * `skills/codeman/reference/endpoints.md` is injected into cases and read by agents
+ * driving Codeman over HTTP. Nothing tied it to the server, so renaming or dropping a
+ * route left the skill confidently telling agents to call a 404. This parses the
+ * `METHOD /api/...` pairs out of the doc and matches them against the `app.<method>()`
+ * registrations in src/web/routes/*.ts.
+ *
+ * Precision over recall on purpose: only a bare uppercase verb followed by an
+ * `/api/...` path counts, so prose that merely mentions a path (the `.../sessions/null`
+ * jq-pitfall example) is ignored, and a spuriously failing guard does not get deleted
+ * by the next person. `/api/v1` is a URL-rewrite alias (server.ts), so the version
+ * segment is dropped before matching, and param NAMES are normalized away since the
+ * doc's `:id` need not match a route's `:sessionId`.
+ *
+ * Port: N/A (pure static analysis).
+ */
+
+import { describe, it, expect } from 'vitest';
+import { readFileSync, readdirSync } from 'node:fs';
+import { fileURLToPath } from 'node:url';
+import { join } from 'node:path';
+
+const HERE = fileURLToPath(new URL('.', import.meta.url));
+const DOC_PATH = join(HERE, '../skills/codeman/reference/endpoints.md');
+const ROUTES_DIR = join(HERE, '../src/web/routes');
+
+/** `METHOD /api/<path>`, stopping before a query string, backtick or prose. */
+const DOC_ENDPOINT = /\b(GET|POST|PUT|PATCH|DELETE)\s+\/(api\/[A-Za-z0-9_:/-]+)/g;
+/** `app.get('/api/…'`, where the path may sit on its own line (case-routes.ts, file-routes.ts). */
+const ROUTE_REGISTRATION = /app\.(get|post|put|patch|delete)\(\s*'([^']+)'/g;
+
+/**
+ * Strip the `/api/v1` alias and replace param names with a placeholder, so
+ * `GET /api/v1/sessions/:id` and `app.get('/api/sessions/:sessionId')` compare equal.
+ */
+function normalize(method: string, path: string): string {
+  const withoutVersion = path.replace(/^\/api\/v1\//, '/api/');
+  const params = withoutVersion.replace(/\/:[^/]+/g, '/:p').replace(/\/$/, '');
+  return `${method.toUpperCase()} ${params}`;
+}
+
+function documentedEndpoints(): string[] {
+  const markdown = readFileSync(DOC_PATH, 'utf-8');
+  const found = new Set<string>();
+  for (const match of markdown.matchAll(DOC_ENDPOINT)) {
+    found.add(normalize(match[1], `/${match[2]}`));
+  }
+  return [...found].sort();
+}
+
+function registeredRoutes(): Set<string> {
+  const registered = new Set<string>();
+  for (const file of readdirSync(ROUTES_DIR)) {
+    if (!file.endsWith('.ts')) continue;
+    const source = readFileSync(join(ROUTES_DIR, file), 'utf-8');
+    for (const match of source.matchAll(ROUTE_REGISTRATION)) {
+      if (!match[2].startsWith('/api/')) continue;
+      registered.add(normalize(match[1], match[2]));
+    }
+  }
+  return registered;
+}
+
+describe('skills/codeman/reference/endpoints.md', () => {
+  it('parses a plausible number of endpoints out of the doc', () => {
+    // A parser that silently matches nothing would make the real assertion below
+    // pass vacuously forever.
+    const documented = documentedEndpoints();
+    expect(documented.length).toBeGreaterThanOrEqual(10);
+    expect(documented).toContain('POST /api/quick-start');
+    expect(documented).toContain('GET /api/sessions/:p/wait');
+  });
+
+  it('finds the route registrations it matches against', () => {
+    const registered = registeredRoutes();
+    expect(registered.size).toBeGreaterThan(100);
+    expect(registered.has('GET /api/status')).toBe(true);
+  });
+
+  it('documents only endpoints that are actually registered', () => {
+    const registered = registeredRoutes();
+    const missing = documentedEndpoints().filter((endpoint) => !registered.has(endpoint));
+    expect(missing).toEqual([]);
+  });
+});

test/cli-skill-target.test.ts

@@ -0,0 +1,144 @@
+/**
+ * @fileoverview Tests for `codeman skill install|uninstall` target resolution
+ * (`resolveCliCasePath` / `resolveSkillTargetPath` in src/cli.ts).
+ *
+ * The linked-cases lookup shipped in 1.14.2 with no guard: before it, `--case`
+ * rejected every case linked in from outside `~/codeman-cases` with "Case not
+ * found" even though the server resolved the same name fine. These tests pin both
+ * halves of that resolution (registry first, cases dir as fallback) and the
+ * tolerance rules around a missing or malformed registry.
+ *
+ * `test/setup.ts` gives this file its own temporary HOME, so `homedir()` and
+ * `dataPath()` already point into a per-file fixture: no os mocking needed.
+ * Port: N/A (pure path resolution, no server).
+ */
+
+import { describe, it, expect, beforeEach, afterEach } from 'vitest';
+import { mkdirSync, rmSync, writeFileSync, existsSync } from 'node:fs';
+import { homedir } from 'node:os';
+import { join } from 'node:path';
+import { dataPath } from '../src/config/instance.js';
+import { program, resolveCliCasePath, resolveSkillTargetPath } from '../src/cli.js';
+
+const LINKED_CASES_FILE = dataPath('linked-cases.json');
+const CASES_DIR = join(homedir(), 'codeman-cases');
+const LINKED_ROOT = join(homedir(), 'elsewhere');
+
+/** Where the packaged skill lands under a case. Mirrors `applyAgentSkill()`. */
+function skillDirIn(casePath: string): string {
+  return join(casePath, '.claude', 'skills', 'codeman');
+}
+
+function writeLinkedCases(content: string): void {
+  mkdirSync(dataPath(), { recursive: true });
+  writeFileSync(LINKED_CASES_FILE, content, 'utf-8');
+}
+
+beforeEach(() => {
+  rmSync(LINKED_CASES_FILE, { force: true });
+  rmSync(CASES_DIR, { recursive: true, force: true });
+  rmSync(LINKED_ROOT, { recursive: true, force: true });
+});
+
+afterEach(() => {
+  rmSync(LINKED_CASES_FILE, { force: true });
+  rmSync(CASES_DIR, { recursive: true, force: true });
+  rmSync(LINKED_ROOT, { recursive: true, force: true });
+});
+
+describe('resolveSkillTargetPath (global)', () => {
+  it('targets the user-scope skill dir when no --case is given', () => {
+    expect(resolveSkillTargetPath({})).toEqual({
+      target: join(homedir(), '.claude', 'skills', 'codeman'),
+    });
+  });
+
+  it('never consults the linked-cases registry for the global target', () => {
+    // A registry entry named after nothing in particular must not divert the
+    // global install, which is not case-scoped at all.
+    writeLinkedCases(JSON.stringify({ anything: join(LINKED_ROOT, 'anything') }));
+    expect(resolveSkillTargetPath({}).target).toBe(join(homedir(), '.claude', 'skills', 'codeman'));
+  });
+});
+
+describe('resolveCliCasePath / resolveSkillTargetPath (--case)', () => {
+  it('resolves a LINKED case through linked-cases.json, not the cases dir', () => {
+    // The 1.14.2 regression: a case linked in from outside ~/codeman-cases was
+    // resolved to a cases-dir path that does not exist, so install refused it.
+    const linkedPath = join(LINKED_ROOT, 'my-repo');
+    mkdirSync(linkedPath, { recursive: true });
+    writeLinkedCases(JSON.stringify({ 'my-repo': linkedPath }));
+
+    expect(resolveCliCasePath('my-repo')).toBe(linkedPath);
+    expect(resolveSkillTargetPath({ case: 'my-repo' })).toEqual({ target: skillDirIn(linkedPath) });
+    expect(existsSync(join(CASES_DIR, 'my-repo'))).toBe(false);
+  });
+
+  it('falls back to the cases dir for a name the registry does not list', () => {
+    const casePath = join(CASES_DIR, 'plain-case');
+    mkdirSync(casePath, { recursive: true });
+    writeLinkedCases(JSON.stringify({ 'other-case': join(LINKED_ROOT, 'other-case') }));
+
+    expect(resolveCliCasePath('plain-case')).toBe(casePath);
+    expect(resolveSkillTargetPath({ case: 'plain-case' })).toEqual({ target: skillDirIn(casePath) });
+  });
+
+  it('reports the resolved path instead of exiting when the case does not exist', () => {
+    // process.exit(1) lives in the CLI wrapper on purpose: calling it here would
+    // kill the test runner.
+    expect(resolveSkillTargetPath({ case: 'nope' })).toEqual({ missingCase: join(CASES_DIR, 'nope') });
+  });
+
+  it('reports the LINKED path when the registry points at a directory that is gone', () => {
+    const linkedPath = join(LINKED_ROOT, 'moved-away');
+    writeLinkedCases(JSON.stringify({ 'moved-away': linkedPath }));
+
+    expect(resolveSkillTargetPath({ case: 'moved-away' })).toEqual({ missingCase: linkedPath });
+  });
+});
+
+describe('linked-cases.json tolerance', () => {
+  const casePath = () => join(CASES_DIR, 'tolerant');
+
+  beforeEach(() => {
+    mkdirSync(casePath(), { recursive: true });
+  });
+
+  it('degrades to the cases dir when the registry file is absent', () => {
+    expect(existsSync(LINKED_CASES_FILE)).toBe(false);
+    expect(resolveSkillTargetPath({ case: 'tolerant' })).toEqual({ target: skillDirIn(casePath()) });
+  });
+
+  it('degrades to the cases dir on malformed JSON rather than throwing', () => {
+    writeLinkedCases('{ not json at all');
+    expect(() => resolveCliCasePath('tolerant')).not.toThrow();
+    expect(resolveSkillTargetPath({ case: 'tolerant' })).toEqual({ target: skillDirIn(casePath()) });
+  });
+
+  it('degrades to the cases dir when the registry is valid JSON of the wrong shape', () => {
+    // A null / array / non-string-valued entry must read as "no linked case",
+    // never as a target path.
+    for (const body of ['null', '[]', JSON.stringify({ tolerant: 42 }), JSON.stringify({ tolerant: '' })]) {
+      writeLinkedCases(body);
+      expect(resolveCliCasePath('tolerant')).toBe(casePath());
+    }
+  });
+});
+
+describe('skill command wiring', () => {
+  it('registers install and uninstall, both accepting --case and --global', () => {
+    const skill = program.commands.find((cmd) => cmd.name() === 'skill');
+    expect(skill).toBeDefined();
+
+    const subcommands = skill!.commands.map((cmd) => cmd.name());
+    expect(subcommands).toEqual(expect.arrayContaining(['install', 'uninstall']));
+
+    for (const name of ['install', 'uninstall']) {
+      const flags = skill!.commands
+        .find((cmd) => cmd.name() === name)!
+        .options.map((opt) => opt.long)
+        .sort();
+      expect(flags).toEqual(['--case', '--global']);
+    }
+  });
+});

test/mocks/mock-route-context.ts

@@ -15,7 +15,7 @@ import { resolveTerminalHistoryConfig } from '../../src/config/terminal-history.
  * Creates a mock context that satisfies all port interfaces.
  * Pre-populated with one session for convenience.
  */
-export function createMockRouteContext(options?: { sessionId?: string }) {
+export function createMockRouteContext(options?: { sessionId?: string; agentSkillEnabled?: boolean }) {
   const sessionId = options?.sessionId ?? 'test-session-1';
   const session = createMockSession(sessionId);
   const sessions = new Map<string, MockSession>();
@@ -86,7 +86,10 @@ 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),
+    // Default OFF mirrors the shipped setting, so existing tests never touch a
+    // case's .claude/skills. Overridable per test because the create-time
+    // injection call sites are otherwise unreachable from a route test.
+    getAgentSkillEnabled: vi.fn(async () => options?.agentSkillEnabled ?? false),
     getDefaultClaudeMdPath: vi.fn(async () => undefined),
     getLightState: vi.fn(() => ({ sessions: [], status: 'ok' })),
     getLightSessionsState: vi.fn(() => {

test/routes/session-routes-agent-skill.test.ts

@@ -0,0 +1,121 @@
+/**
+ * @fileoverview Agent-skill injection on POST /api/sessions (docs/agent-control-plan.md §2).
+ *
+ * The quick-start half of this is covered end-to-end in test/quick-start.test.ts;
+ * the plain create path had NO coverage, because the shared mock context hardcoded
+ * `getAgentSkillEnabled` to false and nothing could flip it. This exercises the real
+ * `applyAgentSkill` against a temp working dir, so it asserts bytes on disk rather
+ * than a spy call.
+ *
+ * Uses app.inject(), so no real HTTP port is needed.
+ */
+
+import { describe, it, expect, beforeEach, afterEach } from 'vitest';
+import Fastify, { type FastifyInstance } from 'fastify';
+import fastifyCookie from '@fastify/cookie';
+import { mkdtemp, rm, readFile } from 'node:fs/promises';
+import { existsSync } from 'node:fs';
+import { join } from 'node:path';
+import { tmpdir } from 'node:os';
+import { createMockRouteContext, type MockRouteContext } from '../mocks/index.js';
+import { installRouteErrorHandler } from '../../src/web/route-error-handler.js';
+import { registerSessionRoutes } from '../../src/web/routes/session-routes.js';
+
+const MARKER_PREFIX = '<!-- codeman-managed-agent-skill';
+
+interface Harness {
+  app: FastifyInstance;
+  ctx: MockRouteContext;
+}
+
+async function createHarness(agentSkillEnabled: boolean): Promise<Harness> {
+  const app = Fastify({ logger: false });
+  await app.register(fastifyCookie);
+  const ctx = createMockRouteContext({ agentSkillEnabled });
+  registerSessionRoutes(app, ctx);
+  installRouteErrorHandler(app);
+  await app.ready();
+  return { app, ctx };
+}
+
+describe('POST /api/sessions agent-skill injection', () => {
+  let workingDir: string;
+  let harness: Harness | undefined;
+
+  const skillDir = () => join(workingDir, '.claude', 'skills', 'codeman');
+
+  beforeEach(async () => {
+    workingDir = await mkdtemp(join(tmpdir(), 'codeman-create-skill-'));
+  });
+
+  afterEach(async () => {
+    await harness?.app.close();
+    harness = undefined;
+    await rm(workingDir, { recursive: true, force: true });
+  });
+
+  it('injects the packaged skill into the working dir when the gate is ON', async () => {
+    harness = await createHarness(true);
+
+    const res = await harness.app.inject({
+      method: 'POST',
+      url: '/api/sessions',
+      payload: { name: 'skill-on', mode: 'claude', workingDir },
+    });
+
+    expect(res.statusCode).toBe(200);
+    expect(harness.ctx.getAgentSkillEnabled).toHaveBeenCalled();
+
+    const skillMd = await readFile(join(skillDir(), 'SKILL.md'), 'utf-8');
+    expect(skillMd.startsWith('---\nname: codeman')).toBe(true);
+    // The marker is what makes the copy ours: without it, uninstall refuses to
+    // remove what this create wrote.
+    expect(skillMd).toContain(MARKER_PREFIX);
+    expect(existsSync(join(skillDir(), 'reference', 'endpoints.md'))).toBe(true);
+  });
+
+  it('leaves the working dir untouched when the gate is OFF', async () => {
+    harness = await createHarness(false);
+
+    const res = await harness.app.inject({
+      method: 'POST',
+      url: '/api/sessions',
+      payload: { name: 'skill-off', mode: 'claude', workingDir },
+    });
+
+    expect(res.statusCode).toBe(200);
+    expect(harness.ctx.getAgentSkillEnabled).toHaveBeenCalled();
+    expect(existsSync(skillDir())).toBe(false);
+  });
+
+  it('does not inject for a non-claude mode even when the gate is ON', async () => {
+    // `.claude/skills/` is read by Claude Code only, so the gate is never even
+    // consulted for the other backends.
+    harness = await createHarness(true);
+
+    const res = await harness.app.inject({
+      method: 'POST',
+      url: '/api/sessions',
+      payload: { name: 'skill-shell', mode: 'shell', workingDir },
+    });
+
+    expect(res.statusCode).toBe(200);
+    expect(harness.ctx.getAgentSkillEnabled).not.toHaveBeenCalled();
+    expect(existsSync(skillDir())).toBe(false);
+  });
+
+  it('defaults an omitted mode to claude and injects', async () => {
+    // The frontend omits `mode` for a plain claude create, so the default branch
+    // is the common path, not an edge case.
+    harness = await createHarness(true);
+
+    const res = await harness.app.inject({
+      method: 'POST',
+      url: '/api/sessions',
+      payload: { name: 'skill-default-mode', workingDir },
+    });
+
+    expect(res.statusCode).toBe(200);
+    expect(existsSync(join(skillDir(), 'SKILL.md'))).toBe(true);
+  });
+});