diff --git a/.changeset/agent-skill-default-on.md b/.changeset/agent-skill-default-on.md new file mode 100644 index 00000000..1ca442b2 --- /dev/null +++ b/.changeset/agent-skill-default-on.md @@ -0,0 +1,5 @@ +--- +'aicodeman': minor +--- + +**The Codeman agent skill is on by default.** New sessions get the skill in their folder, where their CLI looks for skills (`.claude/skills/codeman/` for Claude Code, `.agents/skills/codeman/` for Codex, Gemini CLI, OpenCode, Antigravity, Grok, DeepSeek and OMP; still none for Pi or a plain shell), so an agent can start and drive workers without an install step. The copy now ignores itself in git (a `.gitignore` of `*` inside the copy, written once and never overwritten), so it never shows up as a new file in your repository, and `codeman skill uninstall` removes it with the copy. A `settings.json` without the `agentSkillEnabled` key now means on; an explicit `false` stays off, which includes every install whose App Settings were saved on an earlier version, since that save wrote the old default. Turn it off under App Settings, Agents & CLIs, All agent CLIs, **Codeman Agent Skill**. Turning it off still removes no copy already written: `codeman skill uninstall --case ` does that per case. diff --git a/src/hooks-config.ts b/src/hooks-config.ts index 26fc6986..38eeeb01 100644 --- a/src/hooks-config.ts +++ b/src/hooks-config.ts @@ -1666,7 +1666,10 @@ export function agentSkillDirFor(casePath: string, dir: AgentSkillDir): string { * Add or remove the Codeman agent skill in `//codeman`, where `dir` is the * skills directory the session's CLI scans (default: Claude Code's `.claude/skills`; * callers resolve it with `agentSkillDir()`), mirroring `applyStatusLineConfig`'s - * shape. Gated by the synced `agentSkillEnabled` app setting (default OFF). + * shape. Gated by the synced `agentSkillEnabled` app setting: default ON since 1.50.0 + * (an absent key is on, only an explicit false is off; `getAgentSkillEnabled()` in + * server.ts), which is safe for a user's repository because the copy ignores itself + * in git (`ensureAgentSkillGitignore`). * * Call-site policy is ADD-ONLY on session create (callers pass `enabled: true` or * skip the call), for the statusLine reason: sessions in a repo share one workspace, diff --git a/src/web/ports/config-port.ts b/src/web/ports/config-port.ts index 024b966f..8dcc7aee 100644 --- a/src/web/ports/config-port.ts +++ b/src/web/ports/config-port.ts @@ -17,7 +17,7 @@ export interface ConfigPort { getModelConfig(): Promise<{ defaultModel?: string; agentTypeOverrides?: Record } | null>; getClaudeModeConfig(): Promise<{ claudeMode?: ClaudeMode; allowedTools?: string }>; getTerminalHistoryConfig(): Promise; - /** Synced `agentSkillEnabled` app setting (default OFF); gates per-case agent-skill injection. */ + /** Synced `agentSkillEnabled` app setting (default ON, only an explicit false is off); gates per-case agent-skill injection. */ getAgentSkillEnabled(): Promise; /** Synced `workspaceHooksEnabled` app setting (default ON); gates INSTALLING hooks into a session's workspace. */ getWorkspaceHooksEnabled(): Promise; diff --git a/src/web/public/index.html b/src/web/public/index.html index c55ad232..604039d7 100644 --- a/src/web/public/index.html +++ b/src/web/public/index.html @@ -2941,7 +2941,7 @@
Codeman Agent Skill - Write Codeman's agent skill into each new session's folder, where that CLI looks for skills, so the agent in it can start workers in any installed CLI, send them prompts and wait for results. Every CLI except Pi and a plain shell. + Write Codeman's agent skill into each new session's folder, where that CLI looks for skills, so the agent in it can start workers in any installed CLI, send them prompts and wait for results. Every CLI except Pi and a plain shell. On by default. The copy ignores itself in git, so your repository never lists it as a new file. Off stops new copies but removes none: codeman skill uninstall --case <name> does that.
diff --git a/src/web/public/settings-ui.js b/src/web/public/settings-ui.js index 8b6270b2..214244f8 100644 --- a/src/web/public/settings-ui.js +++ b/src/web/public/settings-ui.js @@ -707,7 +707,9 @@ Object.assign(CodemanApp.prototype, { this._applyCodexSettingsVisibility(); // Claude Permissions settings document.getElementById('appSettingsAgentTeams').checked = settings.agentTeamsEnabled ?? false; - document.getElementById('appSettingsAgentSkill').checked = settings.agentSkillEnabled ?? false; + // Default ON (since 1.50.0), like the workspace hooks below: the server reads an + // absent key as on, so the switch must too, or the next Save would write false. + document.getElementById('appSettingsAgentSkill').checked = settings.agentSkillEnabled !== false; // Default ON: an absent key is a user who has never seen this setting, and OFF // for them means no tab alerts in any workspace Codeman did not scaffold. document.getElementById('appSettingsWorkspaceHooks').checked = settings.workspaceHooksEnabled !== false; diff --git a/src/web/routes/session-routes.ts b/src/web/routes/session-routes.ts index b7ba3f41..a9158eaa 100644 --- a/src/web/routes/session-routes.ts +++ b/src/web/routes/session-routes.ts @@ -978,8 +978,8 @@ export function _paneDeathWatcherCount(): number { * * `applyAgentSkill` declines two shapes rather than writing through them ('foreign': * an unmarked skills/codeman the user authored; 'symlink': the skill dir or its - * parent is a link). Both were silent: the user flips `agentSkillEnabled` on, nothing - * appears in the case, and there is nowhere to look for why. The ordinary outcomes + * parent is a link). Both were silent: `agentSkillEnabled` is on (the default since + * 1.50.0), nothing appears in the case, and there is nowhere to look for why. The ordinary outcomes * ('installed'/'refreshed'/'unchanged') stay unlogged since they would print on every * single session create. * diff --git a/src/web/server.ts b/src/web/server.ts index f9202601..d75a4385 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -2450,11 +2450,15 @@ export class WebServer extends EventEmitter { return resolveTerminalHistoryConfig(settings); } - // Whether the Codeman agent skill is injected into cases on Claude session create - // (synced `agentSkillEnabled` setting, default OFF; docs/agent-control-plan.md §2). + // Whether the Codeman agent skill is injected into a session's workspace on create + // (synced `agentSkillEnabled` setting; docs/agent-control-plan.md §2). Default ON + // since 1.50.0, like `workspaceHooksEnabled` below: an absent key means a user who + // has never touched the switch, and only an explicit false turns it off. The copy + // ignores itself in git (`.gitignore` of `*`, see installAgentSkillInto), so the + // default leaves a user's `git status` clean. private async getAgentSkillEnabled(): Promise { const settings = await this.readSettings(); - return settings.agentSkillEnabled === true; + return settings.agentSkillEnabled !== false; } // Whether pending agent-inbox mail is snapshotted to disk (synced diff --git a/test/agent-skill-setting-ui.test.ts b/test/agent-skill-setting-ui.test.ts new file mode 100644 index 00000000..a62869dc --- /dev/null +++ b/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; +interface SettingsApp { + openAppSettings(): void; + saveAppSettings(): Promise; + loadAppSettingsFromServer(p: Promise): Promise; + _apiPut: ReturnType; +} + +/** 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 '); + }); +}); diff --git a/test/mocks/mock-route-context.ts b/test/mocks/mock-route-context.ts index 70a8133f..641db7e9 100644 --- a/test/mocks/mock-route-context.ts +++ b/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. diff --git a/test/quick-start.test.ts b/test/quick-start.test.ts index 8daeff46..20e1fc56 100644 --- a/test/quick-start.test.ts +++ b/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 { + 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): Promise { + 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> { + const body = (await (await fetch(`${baseUrl}/api/settings`)).json()) as Record; + const settings = (body.success === true ? body.data : body) as Record; + // 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('