feat(agent-skill): turn the Codeman agent skill on by default

An absent `agentSkillEnabled` now means on, on the server
(`getAgentSkillEnabled()` reads `!== false`, like `workspaceHooksEnabled`)
and on the page (the App Settings switch loads `!== false`, so a Save of any
other setting no longer writes the old default back). Only an explicit false
turns it off. A partial settings PUT that omits the key leaves it as stored,
since the route merges the request into the file.

The switch's help text says it is on by default, that the copy stays out of
git, and that turning it off removes no copy already written. The shared
route-test mock now defaults to on too, so route tests see what a user sees.

Tests: quick-start against a real server (absent key injects, a partial PUT
keeps it on, an explicit false stays off through a later partial PUT), the
real openAppSettings/saveAppSettings in JSDOM (absent shows on, explicit false
shows off, a Save on a never-touched install sends true and passes the
schema), and the plain create route at the mock's default.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

test/agent-skill-setting-ui.test.ts

@@ -0,0 +1,191 @@
+/**
+ * @fileoverview The Codeman Agent Skill switch (`agentSkillEnabled`) in App Settings,
+ * on by default since 1.50.0.
+ *
+ * What is pinned, and why it matters:
+ *  - The REAL openAppSettings() reads an ABSENT key as on, like the server's
+ *    getAgentSkillEnabled() (`!== false`). The page and the server must agree: a
+ *    page that showed an absent key as off would write `false` on the next Save of
+ *    any other setting, and the default would be gone without the user touching it.
+ *  - Only an explicit false shows the switch off, wherever it came from (this
+ *    device's blob or the server's settings.json, merged as a synced key).
+ *  - saveAppSettings() sends the switch in the settings PUT, and the body passes the
+ *    .strict() SettingsUpdateSchema (an undeclared key 400s every save).
+ *  - The row says it is on by default, that the copy stays out of git, and how to
+ *    remove copies already written (turning it off sweeps nothing).
+ *
+ * The real modules run INSIDE a JSDOM window (runScripts: 'outside-only'), with the
+ * harness of test/tab-cli-logos-setting.test.ts.
+ *
+ * Port: none.
+ */
+
+import { readFileSync } from 'node:fs';
+import { join } from 'node:path';
+import { JSDOM } from 'jsdom';
+import { afterEach, describe, expect, it, vi } from 'vitest';
+import { SettingsUpdateSchema } from '../src/web/schemas.js';
+
+const PUBLIC = join(process.cwd(), 'src/web/public');
+const read = (name: string) => readFileSync(join(PUBLIC, name), 'utf8');
+const INDEX = read('index.html');
+const SCRIPTS = ['constants.js', 'app.js', 'settings-ui.js'].map(read);
+
+const DESKTOP_KEY = 'codeman-app-settings';
+
+type Settings = Record<string, unknown>;
+interface SettingsApp {
+  openAppSettings(): void;
+  saveAppSettings(): Promise<void>;
+  loadAppSettingsFromServer(p: Promise<Settings>): Promise<Settings>;
+  _apiPut: ReturnType<typeof vi.fn>;
+}
+
+/** Methods that run for real; every other method of the app is a no-op here. */
+const REAL = new Set([
+  'openAppSettings',
+  'saveAppSettings',
+  'loadAppSettingsFromServer',
+  'loadAppSettingsFromStorage',
+  'saveAppSettingsToStorage',
+  'getSettingsStorageKey',
+  'getDefaultSettings',
+  'resolveTabArrangement',
+  'resolveTabStateOrder',
+  // Real so the save builds the body a browser would send (the schema check).
+  'resolveHeaderStatsStyle',
+  'resolveSessionSidebarFontSize',
+  '_displaySettingKeySet',
+]);
+
+/** Methods the load/save paths call that other modules add to the prototype. */
+const ELSEWHERE = [
+  'showToast',
+  '_updateLocalEchoState',
+  'renderProjectInsightsPanel',
+  'updateSubagentWindowVisibility',
+  'showWelcome',
+  '_applyRunMode',
+];
+
+const windows: { close(): void }[] = [];
+afterEach(() => {
+  for (const win of windows.splice(0)) win.close();
+});
+
+/** The whole index.html with constants.js, app.js and settings-ui.js loaded into it. */
+async function boot(stored?: Settings) {
+  const dom = new JSDOM(INDEX, { url: 'http://localhost/', runScripts: 'outside-only' });
+  const win = dom.window as unknown as Window & typeof globalThis & { eval(src: string): void; __App: any };
+  windows.push(win);
+  // After load, so app.js's DOMContentLoaded boot never runs; only the prototype is under test.
+  if (win.document.readyState !== 'complete') await new Promise((resolve) => win.addEventListener('load', resolve));
+  win.setInterval = (() => 0) as unknown as typeof win.setInterval;
+  if (stored) win.localStorage.setItem(DESKTOP_KEY, JSON.stringify(stored));
+  win.eval(
+    [
+      `var MobileDetection = {
+         getDeviceType: () => 'desktop',
+         isHandheldDevice: () => false,
+         isMobile: () => false,
+         isTouchDevice: () => false,
+       };
+       var KeyboardHandler = {}, SwipeHandler = {}, DeepgramProvider = {}, NotificationManager = function () {};
+       var KeyboardAccessoryBar = { setMode() {} };
+       var VoiceInput = { _getDeepgramConfig: () => ({}), _saveDeepgramConfig() {}, refreshClaudeStatus: () => Promise.resolve() };
+       var FocusTrap = function () { this.activate = () => {}; this.deactivate = () => {}; };
+       var DEFAULT_VOICE_KEYTERMS = '';`,
+      ...SCRIPTS,
+      'window.__App = CodemanApp;',
+    ].join('\n')
+  );
+  const target = Object.create(win.__App.prototype);
+  target.sessions = new Map();
+  target.sessionOrder = [];
+  target._apiPut = vi.fn(async () => ({ ok: true }));
+  const noop = () => undefined;
+  for (const name of ELSEWHERE) target[name] = noop;
+  const app = new Proxy(target, {
+    get(t, key, receiver) {
+      const value = Reflect.get(t, key, receiver);
+      const stubbed =
+        typeof key === 'string' && typeof value === 'function' && !Object.hasOwn(t, key) && !REAL.has(key);
+      return stubbed ? noop : value;
+    },
+  }) as SettingsApp;
+  const checkbox = () => win.document.getElementById('appSettingsAgentSkill') as HTMLInputElement;
+  const putBody = () => {
+    const calls = app._apiPut.mock.calls as [string, Settings][];
+    const call = calls.find(([path]) => path === '/api/settings');
+    expect(call, 'a settings PUT').toBeDefined();
+    return call![1];
+  };
+  return { app, checkbox, putBody };
+}
+
+describe('the switch loads on by default', () => {
+  it('an absent key shows on, an explicit false off, an explicit true on', async () => {
+    for (const [stored, expected] of [
+      [{}, true],
+      [{ agentSkillEnabled: false }, false],
+      [{ agentSkillEnabled: true }, true],
+    ] as const) {
+      const { app, checkbox } = await boot(stored);
+      checkbox().checked = !expected;
+      app.openAppSettings();
+      expect(checkbox().checked, JSON.stringify(stored)).toBe(expected);
+    }
+  });
+
+  it('a fresh device on a server with no stored value shows on', async () => {
+    const { app, checkbox } = await boot();
+    await app.loadAppSettingsFromServer(Promise.resolve({ lastUsedCase: 'demo' }));
+    checkbox().checked = false;
+    app.openAppSettings();
+    expect(checkbox().checked).toBe(true);
+  });
+
+  it("the server's explicit false wins over this device's blob (a synced key)", async () => {
+    const { app, checkbox } = await boot({ agentSkillEnabled: true });
+    await app.loadAppSettingsFromServer(Promise.resolve({ agentSkillEnabled: false }));
+    app.openAppSettings();
+    expect(checkbox().checked).toBe(false);
+  });
+});
+
+describe('Save', () => {
+  it('saving other settings on a never-touched install keeps it on, and the body passes the schema', async () => {
+    const { app, putBody } = await boot({});
+    app.openAppSettings();
+    await app.saveAppSettings();
+    const body = putBody();
+    expect(body.agentSkillEnabled).toBe(true);
+    // The whole body, not just this key: an undeclared key 400s every save.
+    expect(SettingsUpdateSchema.safeParse(body).error?.issues ?? []).toEqual([]);
+  });
+
+  it('switching it off sends an explicit false', async () => {
+    const { app, checkbox, putBody } = await boot({});
+    app.openAppSettings();
+    checkbox().checked = false;
+    await app.saveAppSettings();
+    expect(putBody().agentSkillEnabled).toBe(false);
+  });
+});
+
+describe('the App Settings row', () => {
+  const doc = new JSDOM(INDEX).window.document;
+  const row = doc.getElementById('appSettingsAgentSkill')!.closest('.set-row') as HTMLElement;
+  const desc = row.querySelector('.set-row-desc')!.textContent!;
+
+  it('is a synced switch', () => {
+    expect((doc.getElementById('appSettingsAgentSkill') as HTMLInputElement).type).toBe('checkbox');
+    expect(row.closest('.set-group')!.querySelector('.set-scope')!.textContent).toBe('synced');
+  });
+
+  it('says it is on by default, that git does not list the copy, and how to remove copies', () => {
+    expect(desc).toContain('On by default');
+    expect(desc).toContain('ignores itself in git');
+    expect(desc).toContain('codeman skill uninstall --case <name>');
+  });
+});

test/mocks/mock-route-context.ts

@@ -128,10 +128,10 @@ export function createMockRouteContext(options?: {
     getModelConfig: vi.fn(async () => null),
     getClaudeModeConfig: vi.fn(async () => ({})),
     getTerminalHistoryConfig: vi.fn(async () => resolveTerminalHistoryConfig({})),
-    // 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),
+    // Default ON mirrors the shipped setting (since 1.50.0), so a route test sees
+    // what a user sees. Writes land in the test's temp working dir, never in a real
+    // repo. Overridable per test, so the OFF branch stays reachable.
+    getAgentSkillEnabled: vi.fn(async () => options?.agentSkillEnabled ?? true),
     // Default OFF mirrors the shipped setting: no test opens a voice relay by accident.
     getClaudeVoiceEnabled: vi.fn(async () => options?.claudeVoiceEnabled ?? false),
     // Default ON mirrors the shipped setting, so a route test sees what a user sees.

test/quick-start.test.ts

@@ -353,49 +353,63 @@ describe('Agent skill injection (agentSkillEnabled)', () => {
     }
   });
 
-  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`, {
+  // The tests below run in order against one server and one settings.json: absent key
+  // (on), a partial PUT that omits it (still on), then an explicit false (off).
+  const skillDirOf = (caseName: string) => join(CASES_DIR, caseName, '.claude', 'skills', 'codeman');
+
+  async function quickStart(prefix: string, mode?: string): Promise<string> {
+    const caseName = `${prefix}-${Date.now()}`;
+    createdCases.push(caseName);
+    const response = await fetch(`${baseUrl}/api/quick-start`, {
       method: 'POST',
       headers: { 'Content-Type': 'application/json' },
-      body: JSON.stringify({ caseName: offCase }),
+      body: JSON.stringify(mode ? { caseName, mode } : { caseName }),
     });
-    const offData = await offResponse.json();
-    expect(offData.success).toBe(true);
-    expect(existsSync(join(CASES_DIR, offCase, '.claude', 'skills', 'codeman'))).toBe(false);
+    expect((await response.json()).success).toBe(true);
+    return caseName;
+  }
 
-    // 2. The `.strict()` settings schema accepts the new synced key.
-    const putResponse = await fetch(`${baseUrl}/api/settings`, {
+  async function putSettings(body: Record<string, unknown>): Promise<void> {
+    const response = await fetch(`${baseUrl}/api/settings`, {
       method: 'PUT',
       headers: { 'Content-Type': 'application/json' },
-      body: JSON.stringify({ agentSkillEnabled: true }),
+      body: JSON.stringify(body),
     });
-    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.
+    expect((await response.json()).success).toBe(true);
+    // The server's settings read is cached ~2s; outwait it so the next create sees the change.
     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);
+  /** What settings.json holds, read back through GET /api/settings (enveloped or not). */
+  async function storedSettings(): Promise<Record<string, unknown>> {
+    const body = (await (await fetch(`${baseUrl}/api/settings`)).json()) as Record<string, unknown>;
+    const settings = (body.success === true ? body.data : body) as Record<string, unknown>;
+    // Not vacuous: every quick-start above records the case it used.
+    expect(settings).toHaveProperty('lastUsedCase');
+    return settings;
+  }
+
+  it('injects by default: a settings.json without the key means on', async () => {
+    const caseName = await quickStart('test-skill-default');
+    // A quick-start writes lastUsedCase, never this key.
+    expect(await storedSettings()).not.toHaveProperty('agentSkillEnabled');
 
-    const skillDir = join(CASES_DIR, onCase, '.claude', 'skills', 'codeman');
     const { readFileSync } = await import('node:fs');
-    const skillMd = readFileSync(join(skillDir, 'SKILL.md'), 'utf-8');
+    const skillMd = readFileSync(join(skillDirOf(caseName), '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);
+    expect(existsSync(join(skillDirOf(caseName), 'reference', 'endpoints.md'))).toBe(true);
+    expect(existsSync(join(skillDirOf(caseName), 'reference', 'recipes.md'))).toBe(true);
+    // The copy ignores itself in git, so the default leaves a repository's status clean.
+    expect(readFileSync(join(skillDirOf(caseName), '.gitignore'), 'utf-8')).toBe('*\n');
+  }, 30000);
+
+  it('a partial settings PUT that omits the key keeps it on', async () => {
+    await putSettings({ autoNameSessions: false });
+    expect(await storedSettings()).not.toHaveProperty('agentSkillEnabled');
+
+    const caseName = await quickStart('test-skill-partial');
+    expect(existsSync(join(skillDirOf(caseName), 'SKILL.md'))).toBe(true);
   }, 30000);
 
   it('does not inject for shell-mode quick-start even when enabled', async () => {
@@ -410,4 +424,15 @@ describe('Agent skill injection (agentSkillEnabled)', () => {
     expect(data.success).toBe(true);
     expect(existsSync(join(CASES_DIR, shellCase, '.claude', 'skills', 'codeman'))).toBe(false);
   });
+
+  it('an explicit false turns it off, and a later partial PUT leaves it off', async () => {
+    // The `.strict()` settings schema accepts the synced key.
+    await putSettings({ agentSkillEnabled: false });
+    await putSettings({ autoNameSessions: true });
+    expect((await storedSettings()).agentSkillEnabled).toBe(false);
+
+    const caseName = await quickStart('test-skill-off');
+    expect(existsSync(join(CASES_DIR, caseName))).toBe(true);
+    expect(existsSync(skillDirOf(caseName))).toBe(false);
+  }, 30000);
 });

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

@@ -2,8 +2,9 @@
  * @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
+ * the plain create path had NO coverage, because the shared mock context once hardcoded
+ * `getAgentSkillEnabled` to false and nothing could flip it (it now defaults to true,
+ * the shipped default since 1.50.0, and takes an override). This exercises the real
  * `applyAgentSkill` against a temp working dir, so it asserts bytes on disk rather
  * than a spy call.
  *
@@ -81,6 +82,27 @@ describe('POST /api/sessions agent-skill injection', () => {
     // remove what this create wrote.
     expect(skillMd).toContain(MARKER_PREFIX);
     expect(existsSync(join(skillDir(), 'reference', 'endpoints.md'))).toBe(true);
+    // The copy ignores itself, so the default leaves the user's `git status` clean.
+    expect(await readFile(join(skillDir(), '.gitignore'), 'utf-8')).toBe('*\n');
+  });
+
+  it('injects with the shared mock context left at its default (the shipped default is ON)', async () => {
+    const app = Fastify({ logger: false });
+    await app.register(fastifyCookie);
+    const ctx = createMockRouteContext();
+    registerSessionRoutes(app, ctx);
+    installRouteErrorHandler(app);
+    await app.ready();
+    harness = { app, ctx };
+
+    const res = await app.inject({
+      method: 'POST',
+      url: '/api/sessions',
+      payload: { name: 'skill-default', mode: 'claude', workingDir },
+    });
+
+    expect(res.statusCode).toBe(200);
+    expect(existsSync(join(skillDir(), 'SKILL.md'))).toBe(true);
   });
 
   it('leaves the working dir untouched when the gate is OFF', async () => {