diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e05d71f..c7ece421 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,14 +15,18 @@ All notable changes to this project will be documented in this file. See [standa ### 🐛 Bug Fixes +- `teamai pull` now deletes a tombstoned agent under all three render extensions, so the Codex `.toml` and Kiro `.json` copies of a removed agent no longer survive on other machines. The cleanup also runs when the team repo rev is unchanged, so an upgrade reaches machines that already pulled the tombstone with an older CLI. `teamai remove agents ` also honours `enabledAgents` and no longer deletes from excluded tools. Fixes [#576](https://github.com/Tencent/teamai-cli/issues/576). +- `teamai remove rules ` and `teamai remove skills ` now honour `enabledAgents` and leave excluded tools untouched, matching the whitelist `teamai pull` already applies when it cleans up a tombstoned resource. Fixes [#590](https://github.com/Tencent/teamai-cli/issues/590). - `teamai import --cache-status` and `--cache-gc` now expose their existing JSON output through the CLI `--json` option. - Course-correction matching normalizes prompts and keywords to Unicode NFC, so composed and decomposed accents match. Stored prompt summaries and the 60-second correction window are unchanged. Fixes [#573](https://github.com/Tencent/teamai-cli/issues/573). - Course-correction detection matches keywords in space-separated scripts as whole words, so Spanish "segundo" no longer counts as `undo` (for [#564](https://github.com/Tencent/teamai-cli/issues/564)). +- `teamai doctor` no longer assumes TGit before initialization and now exits with code 1 when any diagnostic check fails. - MCP `requires` is resolved from `PATH` (including Windows `PATHEXT`), so `teamai mcp inject` no longer skips servers such as `uvx` on Windows ([#540](https://github.com/Tencent/teamai-cli/pull/540), for [#539](https://github.com/Tencent/teamai-cli/issues/539)). - The GitHub and CNB providers resolve their CLI to a launchable absolute path and start it through cross-spawn, so on Windows they no longer answer "installed" while every call fails silently ([#520](https://github.com/Tencent/teamai-cli/pull/520)). - `enabledAgents` now also gates CLI builtin deploy, CLAUDE.md-class injects, and last-pull skip-sync targets, so an already-installed tool outside the whitelist is not written to ([#510](https://github.com/Tencent/teamai-cli/issues/510)). - `teamai status` counts rule files in subdirectories recursively ([#437](https://github.com/Tencent/teamai-cli/pull/437)). - Codex Stop-phase contribution hints are deferred to the next prompt, so the host no longer rejects `additionalContext` ([#441](https://github.com/Tencent/teamai-cli/pull/441)). +- Agent version detection launches the agent CLI through cross-spawn, so on Windows an npm-installed agent CLI such as `codebuddy`, `claude` or `openclaw` (a `.cmd` shim) reports its version instead of an empty `agent_version`. ### 📝 Documentation diff --git a/docs/usage-guide.md b/docs/usage-guide.md index 55c20a60..b7e522f5 100644 --- a/docs/usage-guide.md +++ b/docs/usage-guide.md @@ -480,7 +480,7 @@ The existing SessionStart hook runs `teamai pull`. When the `packages` declarati ```bash teamai packages # Install every team declaration teamai packages --dry-run # Preview native commands without installing or writing files -teamai doctor # Check runtimes and declared package/marketplace/plugin status +teamai doctor # Check runtimes and declared package/marketplace/plugin status; exits 1 when any check fails ``` After a successful install, TeamAI writes a local snapshot to `teamai.lock` under the active scope's `.teamai` directory. The lock records installed versions and the declaration hash used by the SessionStart hint; it is not stored in the team repository. In user scope, machine-wide npm tools and Claude plugins are acknowledged once, while project npm dependencies are acknowledged separately for each working directory so installing in one repository cannot silence another repository's hint. @@ -1390,7 +1390,7 @@ roles: agents: [common, frontend] # optional; omitted = root-level agents only ``` -`teamai pull` copies these into each Tier-1 tool's `agents/` directory (e.g. `~/.claude/agents/`), flattened by file name, so two active namespaces must not define the same agent name (pull reports the collision and skips the scope). When a member changes role, agents of the namespaces that stopped being active are removed on the next pull, unless the deployed copy was edited locally, in which case it is kept with a warning. Without a configured role, every agent syncs. `teamai push` resolves the source using the same active role and project namespaces as pull. It writes edits to that source and skips ambiguous destinations with a warning; an agent with only inactive sources is also skipped. Skipped agents do not block other resources in the same push. A new agent lands at the root. Cleanup checks each tool separately, respecting YAML `targets` and legacy format support. An active same-named agent protects a deployed file only when it targets that tool and output file. The CLI's built-in `teamai-recall.md` is deployed alongside team agents but is not uploaded by `teamai push`. +`teamai pull` copies these into each Tier-1 tool's `agents/` directory (e.g. `~/.claude/agents/`), flattened by file name, so two active namespaces must not define the same agent name (pull reports the collision and skips the scope). `teamai pull` writes `.toml` for Codex tools, `.json` for Kiro, and `.md` for every other tool. When a member changes role, agents of the namespaces that stopped being active are removed on the next pull, unless the deployed copy was edited locally, in which case it is kept with a warning. Without a configured role, every agent syncs. `teamai push` resolves the source using the same active role and project namespaces as pull. It writes edits to that source and skips ambiguous destinations with a warning; an agent with only inactive sources is also skipped. Skipped agents do not block other resources in the same push. A new agent lands at the root. Cleanup checks each tool separately, respecting YAML `targets` and legacy format support. An active same-named agent protects a deployed file only when it targets that tool and output file. `teamai remove agents ` records a tombstone. The next pull on every other machine deletes `.md`, `.toml` and `.json` from each synced tool's agents directory. That cleanup also runs when the pull finds the team repo unchanged. The CLI's built-in `teamai-recall.md` is deployed alongside team agents but is not uploaded by `teamai push`. ### OpenCode @@ -1460,6 +1460,8 @@ teamai remove agents teamai remove mcp ``` +`teamai doctor` exits with code 0 only when every check passes, and code 1 when any check fails. Before initialization, it reports the missing configuration without assuming a Git provider. + Auto-update runs in the Stop hook and is controlled by two tiers: | Tier | File | Field | Value | @@ -1664,7 +1666,7 @@ Shared resources (the env block, docs directory, and `~/.teamai/`) are removed * The exclusion is durable: `uninstall --agent ` drops the tool from `enabledAgents` and records it in `disabledAgents`, so a later `pull` (or another tool's session-start hook) will not resurrect its skills, rules, agents, CLAUDE.md block, or hooks. Running `init --agent ` again clears the exclusion and re-enables sync for that tool. -The same `enabledAgents` whitelist (from `init --agent`) also gates CLI built-in skills/rules/agents and CLAUDE.md-class injects: an already-installed tool outside the list is not written to, even if its root directory already exists. Editing `enabledAgents` without `init` still invalidates the last-pull skip cache for newly added tools. +The same `enabledAgents` whitelist (from `init --agent`) also gates CLI built-in skills/rules/agents and CLAUDE.md-class injects: an already-installed tool outside the list is neither written to nor deleted from, even if its root directory already exists. `teamai remove` respects the same whitelist for agents, rules, and skills. Editing `enabledAgents` without `init` still invalidates the last-pull skip cache for newly added tools. To rejoin after uninstalling: diff --git a/docs/usage-guide.zh-CN.md b/docs/usage-guide.zh-CN.md index 867c502f..819df5d4 100644 --- a/docs/usage-guide.zh-CN.md +++ b/docs/usage-guide.zh-CN.md @@ -462,7 +462,7 @@ Claude 插件 target 使用 `plugin@marketplace` 格式。`claude-plugins-offici ```bash teamai packages # 安装团队声明的全部包和插件 teamai packages --dry-run # 预览底层命令，不安装也不写文件 -teamai doctor # 检查运行环境及声明的包、marketplace、插件状态 +teamai doctor # 检查运行环境及声明的包、marketplace、插件状态；任一检查失败时退出码为 1 ``` 安装成功后，TeamAI 会在当前 scope 的 `.teamai` 目录下写入本地快照 `teamai.lock`。该文件记录已安装版本，以及供 SessionStart 提示比对的声明哈希，不会写入团队仓库。在 user scope 下，全局 npm 工具和 Claude 插件只需确认一次；项目 npm 依赖会按工作目录分别确认，避免在一个仓库安装后错误关闭另一个仓库的提示。 @@ -1354,7 +1354,7 @@ roles: agents: [common, frontend] # 可选；省略 = 只同步根目录 agents ``` -`teamai pull` 会将它们按文件名拍平复制到每个 Tier-1 工具的 `agents/` 目录（如 `~/.claude/agents/`），因此两个活跃 namespace 不能定义同名 agent（pull 会报告冲突并跳过该 scope）。成员切换角色后，不再活跃的 namespace 中的 agents 会在下一次 pull 时被移除；若本地副本已被手动修改，则保留并给出警告。未配置角色时同步全部 agents。`teamai push` 使用与 pull 相同的活跃角色和项目 namespace 来确定源文件，并将修改写回该源文件；若存在多个候选目标，则跳过并给出警告。若源文件均不活跃，也会跳过。跳过的 agent 不会阻止同一次 push 中的其他资源。新 agent 落在根目录。清理会逐个工具检查 YAML 的 `targets` 和旧格式支持；只有活跃的同名 agent 会写入该工具的同一输出文件时，才保留该文件。CLI 内置的 `teamai-recall.md` 与团队 agents 并列部署，但不会被 `teamai push` 上传。 +`teamai pull` 会将它们按文件名拍平复制到每个 Tier-1 工具的 `agents/` 目录（如 `~/.claude/agents/`），因此两个活跃 namespace 不能定义同名 agent（pull 会报告冲突并跳过该 scope）。`teamai pull` 为 Codex 系工具写入 `.toml`，为 Kiro 写入 `.json`，其余工具写入 `.md`。成员切换角色后，不再活跃的 namespace 中的 agents 会在下一次 pull 时被移除；若本地副本已被手动修改，则保留并给出警告。未配置角色时同步全部 agents。`teamai push` 使用与 pull 相同的活跃角色和项目 namespace 来确定源文件，并将修改写回该源文件；若存在多个候选目标，则跳过并给出警告。若源文件均不活跃，也会跳过。跳过的 agent 不会阻止同一次 push 中的其他资源。新 agent 落在根目录。清理会逐个工具检查 YAML 的 `targets` 和旧格式支持；只有活跃的同名 agent 会写入该工具的同一输出文件时，才保留该文件。`teamai remove agents ` 会记录 tombstone。其他机器下一次 pull 时，会从每个同步中的工具的 agents 目录删除 `.md`、`.toml` 和 `.json`。即使该次 pull 发现团队仓库没有变化，也会执行清理。CLI 内置的 `teamai-recall.md` 与团队 agents 并列部署，但不会被 `teamai push` 上传。 ### OpenCode @@ -1424,6 +1424,8 @@ teamai remove agents teamai remove mcp ``` +仅当所有检查通过时，`teamai doctor` 才以状态码 0 退出；任一检查失败时以状态码 1 退出。尚未初始化时，它只报告缺少配置，不会臆测 Git 托管平台。 + 自动更新在 Stop hook 中执行，可通过两层控制： | 层级 | 文件 | 字段 | 值 | @@ -1620,7 +1622,7 @@ teamai uninstall --agent claude 该排除是持久的：`uninstall --agent ` 会把该工具从 `enabledAgents` 移除并记入 `disabledAgents`，因此之后的 `pull`（或其他工具的 session-start hook）不会再把它的 skills、rules、agents、CLAUDE.md 块或 hooks 重新装回。重新执行 `init --agent ` 会清除该排除、恢复对该工具的同步。 -同一套 `enabledAgents` 白名单（来自 `init --agent`）也约束 CLI 内置 skills/rules/agents 以及 CLAUDE.md 类注入：即使工具根目录已经存在，白名单外的已安装工具也不会被写入。不经过 `init` 直接把工具加进 `enabledAgents` 时，last-pull 跳过缓存会对新加入的工具失效。 +同一套 `enabledAgents` 白名单（来自 `init --agent`）也约束 CLI 内置 skills/rules/agents 以及 CLAUDE.md 类注入：即使工具根目录已经存在，白名单外的已安装工具也不会被写入或删除。`teamai remove` 对 agents、rules 和 skills 同样遵守该白名单。不经过 `init` 直接把工具加进 `enabledAgents` 时，last-pull 跳过缓存会对新加入的工具失效。 卸载后如需重新加入： diff --git a/src/__tests__/agent-version.test.ts b/src/__tests__/agent-version.test.ts index 0daa5277..0229b07b 100644 --- a/src/__tests__/agent-version.test.ts +++ b/src/__tests__/agent-version.test.ts @@ -1,5 +1,8 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { execFile } from 'node:child_process'; +import { chmod, mkdtemp, rm, writeFile } from 'node:fs/promises'; +import os from 'node:os'; +import path from 'node:path'; import { getAgentVersion, clearVersionCache, _readPlistVersion } from '../agent-version.js'; beforeEach(() => { @@ -69,6 +72,35 @@ describe('getAgentVersion', () => { }); }); +describe('getAgentVersion with an npm-installed CLI', () => { + let binDir: string; + let savedPath: string | undefined; + + beforeEach(async () => { + binDir = await mkdtemp(path.join(os.tmpdir(), 'teamai-agent-version-')); + savedPath = process.env.PATH; + // npm writes only `codebuddy.cmd` / `codebuddy.ps1` on Windows, so the + // bare name has no `.exe` for CreateProcess to find. + if (process.platform === 'win32') { + await writeFile(path.join(binDir, 'codebuddy.cmd'), '@echo 2.5.0\r\n'); + } else { + const script = path.join(binDir, 'codebuddy'); + await writeFile(script, '#!/bin/sh\necho 2.5.0\n'); + await chmod(script, 0o755); + } + process.env.PATH = `${binDir}${path.delimiter}${savedPath ?? ''}`; + }); + + afterEach(async () => { + process.env.PATH = savedPath; + await rm(binDir, { recursive: true, force: true }); + }); + + it('reads the version through the platform launcher', async () => { + expect(await getAgentVersion('codebuddy')).toBe('2.5.0'); + }); +}); + describe('_readPlistVersion', () => { it('returns empty string for non-existent path', async () => { const ver = await _readPlistVersion('/nonexistent/App.app'); diff --git a/src/__tests__/agents.test.ts b/src/__tests__/agents.test.ts index 539d5d5c..96179646 100644 --- a/src/__tests__/agents.test.ts +++ b/src/__tests__/agents.test.ts @@ -417,6 +417,18 @@ projects: expect(tombstone.split('\n').map((l) => l.trim())).toContain('old'); }); + it('removeItem leaves agents of an excluded tool alone', async () => { + await fse.writeFile(path.join(repoPath, 'agents', 'old.md'), 'old'); + await fse.writeFile(path.join(homeDir, '.claude/agents', 'old.md'), 'old'); + await fse.writeFile(path.join(homeDir, '.codebuddy/agents', 'old.md'), 'old'); + + // enabledAgents whitelists claude only, so codebuddy is not ours to touch. + await handler.removeItem('old', teamConfig, { ...localConfig, enabledAgents: ['claude'] }); + + expect(await fse.pathExists(path.join(homeDir, '.claude/agents', 'old.md'))).toBe(false); + expect(await fse.pathExists(path.join(homeDir, '.codebuddy/agents', 'old.md'))).toBe(true); + }); + it('removeItem deletes a namespaced agent from the team repo and tombstones it', async () => { await fse.ensureDir(path.join(repoPath, 'agents', 'devops')); await fse.writeFile(path.join(repoPath, 'agents', 'devops', 'tf.yaml'), 'name: tf\n'); diff --git a/src/__tests__/doctor.test.ts b/src/__tests__/doctor.test.ts index 883ab5d0..8be540b3 100644 --- a/src/__tests__/doctor.test.ts +++ b/src/__tests__/doctor.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect, vi, beforeEach, type Mock } from 'vitest'; +import path from 'node:path'; // ── Mocks ──────────────────────────────────────────────── @@ -35,6 +36,7 @@ import { loadLocalConfig, loadTeamConfig } from '../config.js'; import { pathExists, readFileSafe } from '../utils/fs.js'; import { TEAMAI_HOOK_SUBCOMMANDS } from '../hooks.js'; import { log } from '../utils/logger.js'; +import { isGfInstalled, gfIsAuthenticated } from '../providers/tgit/index.js'; import { doctor } from '../doctor.js'; const mockedLoadLocalConfig = loadLocalConfig as Mock; @@ -42,6 +44,8 @@ const mockedLoadTeamConfig = loadTeamConfig as Mock; const mockedPathExists = pathExists as Mock; const mockedReadFileSafe = readFileSafe as Mock; const mockedLog = log as unknown as { info: Mock; success: Mock; warn: Mock; error: Mock; debug: Mock }; +const mockedIsGfInstalled = isGfInstalled as Mock; +const mockedGfIsAuthenticated = gfIsAuthenticated as Mock; const mockLocalConfig = { repo: { localPath: '/tmp/repo', remote: 'https://git.woa.com/team/repo.git' }, @@ -92,12 +96,17 @@ beforeEach(() => { describe('doctor — hook checks', () => { it('should pass when all subcommands are present in settings', async () => { - await doctor({}); + mockedLoadTeamConfig.mockResolvedValue({ + ...mockTeamConfig, + sharing: { env: { injectShellProfile: false } }, + }); + const allPassed = await doctor({}); // Should show the hooks check passing (✔) expect(consoleSpy).toHaveBeenCalledWith( expect.stringContaining('✔'), ); + expect(allPassed).toBe(true); }); it('should fail when a subcommand is missing from settings', async () => { @@ -113,7 +122,7 @@ describe('doctor — hook checks', () => { return null; }); - await doctor({}); + const allPassed = await doctor({}); // Should show the hooks check failing (✖) with fix suggestion expect(consoleSpy).toHaveBeenCalledWith( @@ -122,6 +131,7 @@ describe('doctor — hook checks', () => { expect(consoleSpy).toHaveBeenCalledWith( expect.stringContaining('teamai hooks inject'), ); + expect(allPassed).toBe(false); }); it('should fail when settings file does not exist', async () => { @@ -180,7 +190,7 @@ describe('doctor — hook checks', () => { // Only carries the hooks (committed to the business repo). mockedReadFileSafe.mockImplementation(async (filePath: string) => { if (filePath.includes('settings.json')) { - return filePath.includes(projectRoot) ? buildFullHooksContent() : '{ "hooks": {} }'; + return filePath.includes(path.normalize(projectRoot)) ? buildFullHooksContent() : '{ "hooks": {} }'; } return null; }); @@ -192,7 +202,7 @@ describe('doctor — hook checks', () => { it('should pass env check when env/env.yaml does not exist in team repo', async () => { mockedPathExists.mockImplementation(async (filePath: string) => { - if (filePath.includes('env/env.yaml')) return false; + if (filePath.endsWith(path.join('env', 'env.yaml'))) return false; return true; }); mockedReadFileSafe.mockImplementation(async (filePath: string) => { @@ -282,4 +292,42 @@ describe('doctor — hook checks', () => { // Should still show claude check expect(allCalls.some((msg: string) => msg.includes('claude'))).toBe(true); }); + + it('does not assume a provider before initialization', async () => { + mockedLoadLocalConfig.mockResolvedValue(null); + mockedLoadTeamConfig.mockResolvedValue(null); + + const allPassed = await doctor({}); + + const allLines = consoleSpy.mock.calls.map((c) => String(c[0])); + expect(allLines).toContain(' Scope: not initialized\n'); + expect(allLines).toContain(' ✖ TeamAI is not initialized'); + expect(allLines.some((line) => line.includes('gf CLI'))).toBe(false); + expect(allLines.some((line) => line.includes('hooks in'))).toBe(false); + expect(mockedIsGfInstalled).not.toHaveBeenCalled(); + expect(mockedGfIsAuthenticated).not.toHaveBeenCalled(); + expect(allPassed).toBe(false); + }); + + it('checks hooks only for enabled agents', async () => { + mockedLoadLocalConfig.mockResolvedValue({ + ...mockLocalConfig, + enabledAgents: ['claude'], + }); + mockedLoadTeamConfig.mockResolvedValue({ + ...mockTeamConfig, + sharing: { env: { injectShellProfile: false } }, + toolPaths: { + claude: { settings: '.claude/settings.json', skills: '.claude/skills' }, + codex: { settings: '.codex/hooks.json', skills: '.codex/skills' }, + }, + }); + + const allPassed = await doctor({}); + + const allLines = consoleSpy.mock.calls.map((c) => String(c[0])); + expect(allLines.some((line) => line.includes('hooks in claude settings'))).toBe(true); + expect(allLines.some((line) => line.includes('hooks in codex settings'))).toBe(false); + expect(allPassed).toBe(true); + }); }); diff --git a/src/__tests__/e2e/doctor-cli.test.ts b/src/__tests__/e2e/doctor-cli.test.ts new file mode 100644 index 00000000..b66df65f --- /dev/null +++ b/src/__tests__/e2e/doctor-cli.test.ts @@ -0,0 +1,114 @@ +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { spawnSync } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', '..', '..'); +const CLI = path.join(ROOT, 'dist', 'index.js'); + +describe('teamai doctor CLI (e2e)', () => { + let sandbox: string; + let uninitializedHome: string; + let initializedHome: string; + let missingHookHome: string; + + function runDoctor(home: string) { + return spawnSync(process.execPath, [CLI, 'doctor'], { + cwd: home, + env: { + ...process.env, + HOME: home, + USERPROFILE: home, + FORCE_COLOR: '0', + }, + encoding: 'utf8', + }); + } + + function writeLocalConfig(home: string, repoLocal: string) { + fs.mkdirSync(path.join(home, '.teamai'), { recursive: true }); + fs.mkdirSync(path.join(home, '.claude'), { recursive: true }); + fs.writeFileSync(path.join(home, '.teamai', 'config.yaml'), [ + 'repo:', + ` localPath: ${JSON.stringify(repoLocal)}`, + ' remote: https://example.invalid/team/repo.git', + ' kind: git', + 'username: e2e-user', + 'updatePolicy: skip', + 'scope: user', + 'enabledAgents:', + ' - claude', + ].join('\n')); + } + + beforeAll(() => { + if (!fs.existsSync(CLI)) throw new Error('Run npm run build before the E2E test.'); + + sandbox = fs.mkdtempSync(path.join(os.tmpdir(), 'teamai-doctor-e2e-')); + uninitializedHome = path.join(sandbox, 'uninitialized-home'); + initializedHome = path.join(sandbox, 'initialized-home'); + missingHookHome = path.join(sandbox, 'missing-hook-home'); + const repoLocal = path.join(sandbox, 'team-repo'); + + fs.mkdirSync(uninitializedHome, { recursive: true }); + fs.mkdirSync(repoLocal, { recursive: true }); + writeLocalConfig(initializedHome, repoLocal); + writeLocalConfig(missingHookHome, repoLocal); + fs.writeFileSync(path.join(repoLocal, 'teamai.yaml'), [ + 'team: doctor-e2e', + 'repo: team/repo', + 'provider: git', + 'sharing:', + ' env:', + ' injectShellProfile: false', + 'toolPaths:', + ' claude:', + ' settings: .claude/settings.json', + ' skills: .claude/skills', + ].join('\n')); + fs.writeFileSync( + path.join(initializedHome, '.claude', 'settings.json'), + JSON.stringify({ + hooks: { + SessionStart: [{ hooks: [{ type: 'command', command: 'teamai hook-dispatch' }] }], + }, + }), + ); + }); + + afterAll(() => { + if (sandbox) fs.rmSync(sandbox, { recursive: true, force: true }); + }); + + it('exits 1 without initialization and does not assume TGit', () => { + const result = runDoctor(uninitializedHome); + const output = `${result.stdout}${result.stderr}`; + + expect(result.status, output).toBe(1); + expect(output).toContain('TeamAI is not initialized'); + expect(output).not.toContain('gf CLI'); + }); + + it('exits 0 when every diagnostic passes', () => { + const result = runDoctor(initializedHome); + const output = `${result.stdout}${result.stderr}`; + + expect(result.status, output).toBe(0); + expect(output).toContain('Team repo exists locally'); + expect(output).toContain('Team config (teamai.yaml) is valid'); + expect(output).toContain('teamai hooks in claude settings'); + expect(output).toContain('Env variables injected in shell profile'); + expect(output).toContain('All checks passed!'); + }); + + it('exits 1 when an enabled agent is missing teamai hooks', () => { + const result = runDoctor(missingHookHome); + const output = `${result.stdout}${result.stderr}`; + + expect(result.status, output).toBe(1); + expect(output).toContain('✖ teamai hooks in claude settings'); + expect(output).toContain('Some checks failed. See suggestions above.'); + }); +}); diff --git a/src/__tests__/init-agent-initial-sync.test.ts b/src/__tests__/init-agent-initial-sync.test.ts new file mode 100644 index 00000000..ff471843 --- /dev/null +++ b/src/__tests__/init-agent-initial-sync.test.ts @@ -0,0 +1,139 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import path from 'node:path'; +import os from 'node:os'; +import fse from 'fs-extra'; + +vi.mock('../config.js', () => ({ + loadTeamConfig: vi.fn(), + loadStateForScope: vi.fn().mockResolvedValue({ lastPull: null }), + saveStateForScope: vi.fn(), +})); + +vi.mock('../utils/git.js', () => ({ + pullRepo: vi.fn().mockResolvedValue('Already up to date.'), + getHeadRev: vi.fn().mockResolvedValue('abc1234'), +})); + +vi.mock('../utils/logger.js', () => ({ + log: { + info: vi.fn(), + success: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + dim: vi.fn(), + }, + spinner: vi.fn(() => ({ + start: vi.fn().mockReturnThis(), + succeed: vi.fn().mockReturnThis(), + fail: vi.fn().mockReturnThis(), + warn: vi.fn().mockReturnThis(), + info: vi.fn().mockReturnThis(), + stop: vi.fn().mockReturnThis(), + })), +})); + +import { seedEnabledAgentDirs } from '../known-agents.js'; +import { pullForScope } from '../pull.js'; +import { loadTeamConfig } from '../config.js'; +import type { TeamaiConfig, LocalConfig } from '../types.js'; + +describe('init --agent: directory seeding + initial sync (#574/#585)', () => { + let tmpDir: string; + let homeDir: string; + let repoPath: string; + + beforeEach(async () => { + tmpDir = await fse.mkdtemp(path.join(os.tmpdir(), 'teamai-init-agent-sync-')); + homeDir = path.join(tmpDir, 'home'); + repoPath = path.join(tmpDir, 'team-repo'); + + await fse.ensureDir(path.join(repoPath, 'rules')); + await fse.writeFile(path.join(repoPath, 'rules', 'onboarding.md'), '# Onboarding Rule\n'); + + vi.stubEnv('HOME', homeDir); + }); + + afterEach(async () => { + vi.unstubAllEnvs(); + await fse.remove(tmpDir); + }); + + it('reproduces the fresh-clone scenario from #574: no manual mkdir, no directories exist before init', async () => { + expect(await fse.pathExists(path.join(homeDir, '.claude'))).toBe(false); + }); + + it('seeding then syncing writes real rule files into a freshly-declared --agent tool, with no manual mkdir', async () => { + const teamConfig: TeamaiConfig = { + team: 'test', + description: '', + repo: 'https://example.com/test/repo.git', + provider: 'github', + reviewers: [], + sharing: { + skills: {}, + rules: { enforced: [] }, + docs: { localDir: '' }, + env: { injectShellProfile: true }, + }, + toolPaths: { + claude: { skills: '.claude/skills', rules: '.claude/rules' }, + }, + }; + + const localConfig: LocalConfig = { + repo: { localPath: repoPath, remote: 'https://example.com/test/repo.git' }, + username: 'testuser', + updatePolicy: 'auto', + additionalRoles: [], + scope: 'user', + enabledAgents: ['claude'], + }; + + vi.mocked(loadTeamConfig).mockResolvedValue(teamConfig); + + // Step 1: this is what init/initHttp now call — seed the declared agent's dir. + const seeded = await seedEnabledAgentDirs(localConfig, teamConfig); + expect(seeded).toEqual(['claude']); + expect(await fse.pathExists(path.join(homeDir, '.claude/skills'))).toBe(true); + + // Step 2: this is the "initial sync" half — pullForScope actually writes files + // now that the dir exists, instead of leaving it empty until a hook fires. + await pullForScope(localConfig, {}); + + expect(await fse.pathExists(path.join(homeDir, '.claude/rules/onboarding.md'))).toBe(true); + }); + + it('init without --agent (enabledAgents unset) creates nothing — regression guard from #585 acceptance criteria', async () => { + const teamConfig: TeamaiConfig = { + team: 'test', + description: '', + repo: 'https://example.com/test/repo.git', + provider: 'github', + reviewers: [], + sharing: { + skills: {}, + rules: { enforced: [] }, + docs: { localDir: '' }, + env: { injectShellProfile: true }, + }, + toolPaths: { + claude: { skills: '.claude/skills', rules: '.claude/rules' }, + }, + }; + + const localConfig: LocalConfig = { + repo: { localPath: repoPath, remote: 'https://example.com/test/repo.git' }, + username: 'testuser', + updatePolicy: 'auto', + additionalRoles: [], + scope: 'user', + // enabledAgents deliberately omitted — no --agent was passed. + }; + + const seeded = await seedEnabledAgentDirs(localConfig, teamConfig); + + expect(seeded).toEqual([]); + expect(await fse.pathExists(path.join(homeDir, '.claude'))).toBe(false); + }); +}); diff --git a/src/__tests__/pull-rules-sync-count.test.ts b/src/__tests__/pull-rules-sync-count.test.ts new file mode 100644 index 00000000..cfc0fb99 --- /dev/null +++ b/src/__tests__/pull-rules-sync-count.test.ts @@ -0,0 +1,350 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import path from 'node:path'; +import os from 'node:os'; +import fse from 'fs-extra'; + +vi.mock('../config.js', () => ({ + requireInit: vi.fn(), + loadState: vi.fn().mockResolvedValue({ lastPull: null }), + saveState: vi.fn(), + loadLocalConfigForScope: vi.fn(), + loadTeamConfig: vi.fn(), + detectProjectConfig: vi.fn().mockResolvedValue(null), + loadStateForScope: vi.fn().mockResolvedValue({ lastPull: null }), + saveStateForScope: vi.fn(), +})); + +vi.mock('../utils/git.js', () => ({ + pullRepo: vi.fn().mockResolvedValue('Already up to date.'), +})); + +vi.mock('../utils/logger.js', () => ({ + log: { + info: vi.fn(), + success: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + dim: vi.fn(), + }, + spinner: vi.fn(() => ({ + start: vi.fn().mockReturnThis(), + succeed: vi.fn().mockReturnThis(), + fail: vi.fn().mockReturnThis(), + warn: vi.fn().mockReturnThis(), + info: vi.fn().mockReturnThis(), + stop: vi.fn().mockReturnThis(), + })), +})); + +vi.mock('../update.js', () => ({ + acquireLock: vi.fn().mockResolvedValue(true), + releaseLock: vi.fn().mockResolvedValue(undefined), +})); + +import { pull } from '../pull.js'; +import { loadLocalConfigForScope, loadTeamConfig, detectProjectConfig } from '../config.js'; +import { log } from '../utils/logger.js'; +import type { TeamaiConfig, LocalConfig } from '../types.js'; + +describe('pull — rules sync count reflects actual writes (#574/#585)', () => { + let tmpDir: string; + let homeDir: string; + let repoPath: string; + + function config(): LocalConfig { + return { + repo: { localPath: repoPath, remote: 'https://example.com/test/repo.git' }, + username: 'testuser', + updatePolicy: 'auto', + additionalRoles: [], + scope: 'user', + }; + } + + beforeEach(async () => { + tmpDir = await fse.mkdtemp(path.join(os.tmpdir(), 'teamai-pull-rules-count-')); + homeDir = path.join(tmpDir, 'home'); + repoPath = path.join(tmpDir, 'team-repo'); + + await fse.ensureDir(path.join(repoPath, 'rules')); + await fse.writeFile(path.join(repoPath, 'rules', 'my-rule.md'), '# My Rule\n'); + + vi.stubEnv('HOME', homeDir); + + const teamConfig: TeamaiConfig = { + team: 'test', + description: '', + repo: 'https://example.com/test/repo.git', + provider: 'github', + reviewers: [], + sharing: { + skills: {}, + rules: { enforced: [] }, + docs: { localDir: '' }, + env: { injectShellProfile: true }, + }, + toolPaths: { + claude: { skills: '.claude/skills', rules: '.claude/rules' }, + }, + }; + + vi.mocked(loadLocalConfigForScope).mockResolvedValue(config()); + vi.mocked(loadTeamConfig).mockResolvedValue(teamConfig); + vi.mocked(detectProjectConfig).mockResolvedValue(null); + vi.mocked(log.success).mockClear(); + vi.mocked(log.warn).mockClear(); + }); + + afterEach(async () => { + vi.unstubAllEnvs(); + await fse.remove(tmpDir); + }); + + it('does NOT claim success when the only configured tool is not installed', async () => { + // Deliberately do NOT create .claude — reproduces #574's exact scenario. + await pull({}); + + expect(await fse.pathExists(path.join(homeDir, '.claude'))).toBe(false); + + const successCalls = vi.mocked(log.success).mock.calls.map(([msg]) => String(msg)); + expect(successCalls.some((m) => /Synced \d+ rule\(s\)/.test(m))).toBe(false); + + const warnCalls = vi.mocked(log.warn).mock.calls.map(([msg]) => String(msg)); + expect(warnCalls.some((m) => /rule\(s\) available but no installed tool directory found/.test(m))).toBe(true); + }); + + it('DOES report success and writes the file when the tool is installed', async () => { + await fse.ensureDir(path.join(homeDir, '.claude')); + + await pull({}); + + expect(await fse.pathExists(path.join(homeDir, '.claude/rules/my-rule.md'))).toBe(true); + + const successCalls = vi.mocked(log.success).mock.calls.map(([msg]) => String(msg)); + expect(successCalls.some((m) => /Synced 1 rule\(s\)/.test(m))).toBe(true); + }); +}); + + +describe('pull — --force bypasses isToolInstalled for rules (#574/#585)', () => { + let tmpDir: string; + let homeDir: string; + let repoPath: string; + + function config(force?: boolean): LocalConfig { + return { + repo: { localPath: repoPath, remote: 'https://example.com/test/repo.git' }, + username: 'testuser', + updatePolicy: 'auto', + additionalRoles: [], + scope: 'user', + }; + } + + beforeEach(async () => { + tmpDir = await fse.mkdtemp(path.join(os.tmpdir(), 'teamai-pull-force-')); + homeDir = path.join(tmpDir, 'home'); + repoPath = path.join(tmpDir, 'team-repo'); + + await fse.ensureDir(path.join(repoPath, 'rules')); + await fse.writeFile(path.join(repoPath, 'rules', 'my-rule.md'), '# My Rule\n'); + + vi.stubEnv('HOME', homeDir); + + const teamConfig: TeamaiConfig = { + team: 'test', + description: '', + repo: 'https://example.com/test/repo.git', + provider: 'github', + reviewers: [], + sharing: { + skills: {}, + rules: { enforced: [] }, + docs: { localDir: '' }, + env: { injectShellProfile: true }, + }, + toolPaths: { + claude: { skills: '.claude/skills', rules: '.claude/rules' }, + }, + }; + + vi.mocked(loadLocalConfigForScope).mockResolvedValue(config()); + vi.mocked(loadTeamConfig).mockResolvedValue(teamConfig); + vi.mocked(detectProjectConfig).mockResolvedValue(null); + vi.mocked(log.success).mockClear(); + vi.mocked(log.warn).mockClear(); + }); + + afterEach(async () => { + vi.unstubAllEnvs(); + await fse.remove(tmpDir); + }); + + it('without --force: does NOT create the missing .claude dir (unchanged prior behavior)', async () => { + await pull({}); + expect(await fse.pathExists(path.join(homeDir, '.claude'))).toBe(false); + }); + + it('with --force: creates the missing .claude dir and writes the rule file', async () => { + expect(await fse.pathExists(path.join(homeDir, '.claude'))).toBe(false); + + await pull({ force: true }); + + expect(await fse.pathExists(path.join(homeDir, '.claude/rules/my-rule.md'))).toBe(true); + + const successCalls = vi.mocked(log.success).mock.calls.map(([msg]) => String(msg)); + expect(successCalls.some((m) => /Synced 1 rule\(s\)/.test(m))).toBe(true); + }); +}); + +describe('pull — same phantom-success gate applied to skills and agents (#574/#585 follow-up)', () => { + let tmpDir: string; + let homeDir: string; + let repoPath: string; + + function config(): LocalConfig { + return { + repo: { localPath: repoPath, remote: 'https://example.com/test/repo.git' }, + username: 'testuser', + updatePolicy: 'auto', + additionalRoles: [], + scope: 'user', + }; + } + + beforeEach(async () => { + tmpDir = await fse.mkdtemp(path.join(os.tmpdir(), 'teamai-pull-skills-agents-count-')); + homeDir = path.join(tmpDir, 'home'); + repoPath = path.join(tmpDir, 'team-repo'); + + await fse.ensureDir(path.join(repoPath, 'skills', 'my-skill')); + await fse.writeFile(path.join(repoPath, 'skills', 'my-skill', 'SKILL.md'), '# My Skill\n'); + + vi.stubEnv('HOME', homeDir); + + const teamConfig: TeamaiConfig = { + team: 'test', + description: '', + repo: 'https://example.com/test/repo.git', + provider: 'github', + reviewers: [], + sharing: { + skills: {}, + rules: { enforced: [] }, + docs: { localDir: '' }, + env: { injectShellProfile: true }, + }, + toolPaths: { + claude: { skills: '.claude/skills', rules: '.claude/rules' }, + }, + }; + + vi.mocked(loadLocalConfigForScope).mockResolvedValue(config()); + vi.mocked(loadTeamConfig).mockResolvedValue(teamConfig); + vi.mocked(detectProjectConfig).mockResolvedValue(null); + vi.mocked(log.success).mockClear(); + vi.mocked(log.warn).mockClear(); + }); + + afterEach(async () => { + vi.unstubAllEnvs(); + await fse.remove(tmpDir); + }); + + it('skills: does NOT claim success when no tool is installed', async () => { + await pull({}); + + expect(await fse.pathExists(path.join(homeDir, '.claude'))).toBe(false); + + const successCalls = vi.mocked(log.success).mock.calls.map(([msg]) => String(msg)); + expect(successCalls.some((m) => /Synced \d+ skills/.test(m))).toBe(false); + + const warnCalls = vi.mocked(log.warn).mock.calls.map(([msg]) => String(msg)); + expect(warnCalls.some((m) => /skills available but no installed tool directory found/.test(m))).toBe(true); + }); + + it('skills: DOES sync and writes the file when the tool is installed', async () => { + await fse.ensureDir(path.join(homeDir, '.claude')); + + await pull({}); + + expect(await fse.pathExists(path.join(homeDir, '.claude/skills/my-skill/SKILL.md'))).toBe(true); + }); +}); + +describe('pull — success message names which tools actually received the write (review follow-up)', () => { + let tmpDir: string; + let homeDir: string; + let repoPath: string; + + function config(): LocalConfig { + return { + repo: { localPath: repoPath, remote: 'https://example.com/test/repo.git' }, + username: 'testuser', + updatePolicy: 'auto', + additionalRoles: [], + scope: 'user', + }; + } + + beforeEach(async () => { + tmpDir = await fse.mkdtemp(path.join(os.tmpdir(), 'teamai-pull-partial-install-')); + homeDir = path.join(tmpDir, 'home'); + repoPath = path.join(tmpDir, 'team-repo'); + + // agents (not skills) hits the plain log.success line the reviewer's diff + // touched — skills always goes through logSyncDetail instead. + await fse.ensureDir(path.join(repoPath, 'agents')); + await fse.writeFile(path.join(repoPath, 'agents', 'my-agent.yaml'), 'name: my-agent\ndescription: test\ninstructions: test\n'); + + // claude installed, codex is not — the exact partial-install scenario. + await fse.ensureDir(path.join(homeDir, '.claude')); + + vi.stubEnv('HOME', homeDir); + + const teamConfig: TeamaiConfig = { + team: 'test', + description: '', + repo: 'https://example.com/test/repo.git', + provider: 'github', + reviewers: [], + sharing: { + skills: {}, + rules: { enforced: [] }, + docs: { localDir: '' }, + env: { injectShellProfile: true }, + }, + toolPaths: { + claude: { skills: '.claude/skills', rules: '.claude/rules', agents: '.claude/agents' }, + codex: { skills: '.codex/skills', rules: '.codex/rules', agents: '.codex/agents' }, + }, + }; + + vi.mocked(loadLocalConfigForScope).mockResolvedValue(config()); + vi.mocked(loadTeamConfig).mockResolvedValue(teamConfig); + vi.mocked(detectProjectConfig).mockResolvedValue(null); + vi.mocked(log.success).mockClear(); + }); + + afterEach(async () => { + vi.unstubAllEnvs(); + await fse.remove(tmpDir); + }); + + it('names only claude, not codex, when only claude is installed', async () => { + await pull({}); + + // claude actually got the file. + expect(await fse.pathExists(path.join(homeDir, '.claude/agents/my-agent.md'))).toBe(true); + // codex silently got nothing — this is the exact gap the reviewer flagged. + expect(await fse.pathExists(path.join(homeDir, '.codex'))).toBe(false); + + const successCalls = vi.mocked(log.success).mock.calls.map(([msg]) => String(msg)); + const agentsMsg = successCalls.find((m) => /Synced \d+ agents/.test(m)); + + expect(agentsMsg).toBeDefined(); + expect(agentsMsg).toContain('claude'); + expect(agentsMsg).not.toContain('codex'); + }); +}); diff --git a/src/__tests__/pull-tombstone.test.ts b/src/__tests__/pull-tombstone.test.ts index 9000acaa..89487e5d 100644 --- a/src/__tests__/pull-tombstone.test.ts +++ b/src/__tests__/pull-tombstone.test.ts @@ -15,8 +15,14 @@ vi.mock('../config.js', () => ({ saveStateForScope: vi.fn(), })); +/** The rev `refreshTeamRepo` resolves for the fake team repo in these tests. */ +const { HEAD_REV } = vi.hoisted(() => ({ HEAD_REV: 'rev-unchanged' })); + vi.mock('../utils/git.js', () => ({ pullRepo: vi.fn().mockResolvedValue('Already up to date.'), + // Needed by the unchanged-rev fast path: without a rev, pull always does a + // full sync and that branch is unreachable. + getHeadRev: vi.fn().mockResolvedValue(HEAD_REV), })); vi.mock('../utils/logger.js', () => ({ @@ -39,7 +45,7 @@ vi.mock('../utils/logger.js', () => ({ })); import { pull, cleanupInactiveNamespaceSkills } from '../pull.js'; -import { loadLocalConfigForScope, loadTeamConfig, detectProjectConfig } from '../config.js'; +import { loadLocalConfigForScope, loadTeamConfig, detectProjectConfig, loadStateForScope } from '../config.js'; import type { TeamaiConfig, LocalConfig } from '../types.js'; vi.mock('../roles.js', () => ({ @@ -153,6 +159,13 @@ describe('pull role-aware sync and cleanup', () => { vi.mocked(loadLocalConfigForScope).mockResolvedValue(localConfig); vi.mocked(loadTeamConfig).mockResolvedValue(teamConfig); vi.mocked(detectProjectConfig).mockResolvedValue(null); + // No stored rev by default, so every test does a full sync unless it opts + // into the unchanged-rev fast path. A fresh object per call, like the real + // loader: pull writes the rev onto what it reads, and a shared object would + // leak that into the next pull of the same test. + vi.mocked(loadStateForScope).mockImplementation( + async () => ({ lastPull: null }) as Awaited>, + ); }); afterEach(async () => { @@ -190,6 +203,77 @@ describe('pull role-aware sync and cleanup', () => { expect(await fse.pathExists(path.join(homeDir, '.codex/skills/old-skill'))).toBe(false); }); + /** Deploys agents to the three tools whose render formats differ (#576). */ + const useAgentToolPaths = (): void => { + vi.mocked(loadTeamConfig).mockResolvedValue({ + team: 'test', + description: '', + repo: 'https://git.woa.com/test/repo.git', + provider: 'tgit' as const, + reviewers: [], + sharing: { + skills: {}, + rules: { enforced: [] }, + docs: { localDir: '' }, + env: { injectShellProfile: true }, + }, + toolPaths: { + claude: { agents: '.claude/agents' }, + codex: { agents: '.codex/agents' }, + kiro: { agents: '.kiro/agents' }, + }, + }); + }; + + /** Writes a tombstone for `foo` plus one stale render per tool. */ + const seedTombstonedAgent = async (): Promise => { + await fse.ensureDir(path.join(repoPath, 'agents')); + await fse.writeFile(path.join(repoPath, 'agents', '.removed'), 'foo\n'); + + for (const [dir, file] of [ + ['.claude/agents', 'foo.md'], + ['.codex/agents', 'foo.toml'], + ['.kiro/agents', 'foo.json'], + ]) { + await fse.ensureDir(path.join(homeDir, dir)); + await fse.writeFile(path.join(homeDir, dir, file), 'stale'); + } + }; + + const expectAgentRendersGone = async (): Promise => { + expect(await fse.pathExists(path.join(homeDir, '.claude/agents', 'foo.md'))).toBe(false); + expect(await fse.pathExists(path.join(homeDir, '.codex/agents', 'foo.toml'))).toBe(false); + expect(await fse.pathExists(path.join(homeDir, '.kiro/agents', 'foo.json'))).toBe(false); + }; + + it('should clean up tombstoned agent renders under every native extension', async () => { + // Regression for #576: the tool-side render extension varies (.md Claude, + // .toml Codex, .json Kiro), so the tombstone pass must clear all of them. + useAgentToolPaths(); + await seedTombstonedAgent(); + + await pull({}); + + await expectAgentRendersGone(); + }); + + it('should clean up tombstoned agents even when the repo rev is unchanged', async () => { + // Regression for #576 on the upgrade path: a machine that pulled the + // tombstone with the older CLI keeps the copies that CLI could not delete, + // and its stored rev never moves again, so the fast path must clean too. + useAgentToolPaths(); + await seedTombstonedAgent(); + vi.mocked(loadStateForScope).mockImplementation(async () => ({ + lastPull: null, + lastPullRev: HEAD_REV, + lastPullTargets: ['claude', 'codex', 'kiro'], + }) as Awaited>); + + await pull({}); + + await expectAgentRendersGone(); + }); + it('should not delete files that are NOT tombstoned', async () => { // No tombstone files await fse.writeFile(path.join(homeDir, '.claude/rules', 'keep-rule.md'), '# Keep'); diff --git a/src/__tests__/remove.test.ts b/src/__tests__/remove.test.ts index 4c6cb1ce..5e74c13b 100644 --- a/src/__tests__/remove.test.ts +++ b/src/__tests__/remove.test.ts @@ -106,6 +106,24 @@ scope: 'user', expect(await fse.pathExists(path.join(homeDir, '.claude-internal', 'rules', 'my-rule.md'))).toBe(false); }); + it('should leave rules of an excluded tool alone', async () => { + // Regression for #590: a tool outside enabledAgents is not ours to write + // to, so it is not ours to delete from either. + await fse.writeFile(path.join(localConfig.repo.localPath, 'rules', 'my-rule.md'), 'rule content'); + // A second team rule keeps `pullAllRules` from returning early, so the + // stale-file pass it runs after the removal is exercised too. + await fse.writeFile(path.join(localConfig.repo.localPath, 'rules', 'other-rule.md'), 'other content'); + await fse.writeFile(path.join(homeDir, '.claude', 'rules', 'my-rule.md'), 'rule content'); + await fse.writeFile(path.join(homeDir, '.codex', 'rules', 'my-rule.md'), 'rule content'); + + const removed = await handler.removeItem('my-rule', teamConfig, { ...localConfig, enabledAgents: ['claude'] }); + + expect(await fse.pathExists(path.join(homeDir, '.claude', 'rules', 'my-rule.md'))).toBe(false); + expect(await fse.pathExists(path.join(homeDir, '.codex', 'rules', 'my-rule.md'))).toBe(true); + // `removed` is what the CLI reports back, so it must not name the excluded tool. + expect(removed.some((p) => p.includes('.codex'))).toBe(false); + }); + it('should only remove from locations where the rule exists', async () => { // Only create in team repo and one tool await fse.writeFile(path.join(localConfig.repo.localPath, 'rules', 'partial-rule.md'), 'content'); @@ -212,6 +230,27 @@ scope: 'user', expect(await fse.pathExists(codexSkill)).toBe(false); }); + it('should leave skills of an excluded tool alone', async () => { + // Regression for #590, the skills half: same gate as rules and agents. + const teamSkill = path.join(localConfig.repo.localPath, 'skills', 'my-skill'); + await fse.ensureDir(teamSkill); + await fse.writeFile(path.join(teamSkill, 'SKILL.md'), '# My Skill'); + + const claudeSkill = path.join(homeDir, '.claude', 'skills', 'my-skill'); + await fse.ensureDir(claudeSkill); + await fse.writeFile(path.join(claudeSkill, 'SKILL.md'), '# My Skill'); + + const codexSkill = path.join(homeDir, '.codex', 'skills', 'my-skill'); + await fse.ensureDir(codexSkill); + await fse.writeFile(path.join(codexSkill, 'SKILL.md'), '# My Skill'); + + const removed = await handler.removeItem('my-skill', teamConfig, { ...localConfig, enabledAgents: ['claude'] }); + + expect(await fse.pathExists(claudeSkill)).toBe(false); + expect(await fse.pathExists(codexSkill)).toBe(true); + expect(removed.some((p) => p.includes('.codex'))).toBe(false); + }); + it('should only remove from locations where the skill exists', async () => { const teamSkill = path.join(localConfig.repo.localPath, 'skills', 'partial-skill'); await fse.ensureDir(teamSkill); diff --git a/src/__tests__/self-mode-agents.test.ts b/src/__tests__/self-mode-agents.test.ts index 3358d148..bfe683f5 100644 --- a/src/__tests__/self-mode-agents.test.ts +++ b/src/__tests__/self-mode-agents.test.ts @@ -10,7 +10,7 @@ vi.mock('../utils/logger.js', () => ({ import { normalizeAgentList, detectHomeInstalledAgents, - seedSelfModeToolDirs, + seedEnabledAgentDirs, SELF_MODE_AGENT_CHOICES, } from '../known-agents.js'; import { resolveSelfModeSelection } from '../init.js'; @@ -74,7 +74,7 @@ describe('detectHomeInstalledAgents', () => { }); }); -describe('seedSelfModeToolDirs (no hardcoded claude default)', () => { +describe('seedEnabledAgentDirs (no hardcoded claude default)', () => { let tmp: string; let repoRoot: string; let teamConfig: TeamaiConfig; @@ -110,26 +110,26 @@ describe('seedSelfModeToolDirs (no hardcoded claude default)', () => { }); it('seeds nothing when enabledAgents is empty (no default claude)', async () => { - const seeded = await seedSelfModeToolDirs(makeConfig([]), teamConfig); + const seeded = await seedEnabledAgentDirs(makeConfig([]), teamConfig); expect(seeded).toEqual([]); expect(await fse.pathExists(path.join(repoRoot, '.claude'))).toBe(false); }); it('seeds nothing when enabledAgents is undefined (no default claude)', async () => { - const seeded = await seedSelfModeToolDirs(makeConfig(undefined), teamConfig); + const seeded = await seedEnabledAgentDirs(makeConfig(undefined), teamConfig); expect(seeded).toEqual([]); expect(await fse.pathExists(path.join(repoRoot, '.claude'))).toBe(false); }); it('seeds exactly the enabled agents, and no others', async () => { - const seeded = await seedSelfModeToolDirs(makeConfig(['codex']), teamConfig); + const seeded = await seedEnabledAgentDirs(makeConfig(['codex']), teamConfig); expect(seeded).toEqual(['codex']); expect(await fse.pathExists(path.join(repoRoot, '.codex/skills'))).toBe(true); expect(await fse.pathExists(path.join(repoRoot, '.claude'))).toBe(false); }); it('seeds multiple selected agents', async () => { - const seeded = await seedSelfModeToolDirs(makeConfig(['claude', 'cursor']), teamConfig); + const seeded = await seedEnabledAgentDirs(makeConfig(['claude', 'cursor']), teamConfig); expect(new Set(seeded)).toEqual(new Set(['claude', 'cursor'])); expect(await fse.pathExists(path.join(repoRoot, '.claude/skills'))).toBe(true); expect(await fse.pathExists(path.join(repoRoot, '.cursor/skills'))).toBe(true); @@ -138,7 +138,7 @@ describe('seedSelfModeToolDirs (no hardcoded claude default)', () => { it('never seeds an explicitly disabled agent', async () => { const config = makeConfig(['claude', 'codex']); config.disabledAgents = ['codex']; - const seeded = await seedSelfModeToolDirs(config, teamConfig); + const seeded = await seedEnabledAgentDirs(config, teamConfig); expect(seeded).toEqual(['claude']); expect(await fse.pathExists(path.join(repoRoot, '.codex'))).toBe(false); }); diff --git a/src/agent-version.ts b/src/agent-version.ts index 67ce0aa8..a2aa4a0d 100644 --- a/src/agent-version.ts +++ b/src/agent-version.ts @@ -7,23 +7,17 @@ * - Fallback: return '' when detection fails (best-effort, never throws). */ -import { execFile } from 'node:child_process'; import { readFile } from 'node:fs/promises'; import path from 'node:path'; import { log } from './utils/logger.js'; +import { probeBinary } from './utils/exec.js'; const VERSION_CACHE = new Map(); +// Launch through cross-spawn (probeBinary): on Windows npm installs these CLIs +// as `.cmd` shims, which the native execFile cannot find or start. async function execVersion(bin: string, args: string[] = ['--version']): Promise { - return new Promise((resolve) => { - execFile(bin, args, { timeout: 5000 }, (err, stdout) => { - if (err) { - resolve(''); - return; - } - resolve(stdout.trim()); - }); - }); + return probeBinary(bin, args); } async function readPlistVersion(appPath: string): Promise { diff --git a/src/bootstrap.ts b/src/bootstrap.ts index 885a7655..be6527ae 100644 --- a/src/bootstrap.ts +++ b/src/bootstrap.ts @@ -191,8 +191,8 @@ export async function bootstrapSelfRepo( // Seed the tool skills-dir so hooks + skills inject on this fresh clone // (isToolInstalled would otherwise skip everything — no /.claude yet). try { - const { seedSelfModeToolDirs } = await import('./known-agents.js'); - await seedSelfModeToolDirs(localConfig, teamConfig); + const { seedEnabledAgentDirs } = await import('./known-agents.js'); + await seedEnabledAgentDirs(localConfig, teamConfig); } catch (e) { log.debug(`[bootstrap] tool-dir seeding skipped: ${(e as Error).message}`); } diff --git a/src/builtin-agents.ts b/src/builtin-agents.ts index 82ca3d0f..d0634cf1 100644 --- a/src/builtin-agents.ts +++ b/src/builtin-agents.ts @@ -7,7 +7,7 @@ import type { TeamaiConfig, LocalConfig } from './types.js'; import { resolveBaseDir, isAgentExcluded, scopedToolPaths } from './types.js'; import { ResourceHandler } from './resources/base.js'; import { getUserHome } from './utils/home.js'; -import { ALL_SUPPORTED_TOOLS, renderForTool, reverseFromClaude } from './resources/agent-format.js'; +import { ALL_SUPPORTED_TOOLS, agentStemFromFilename, renderForTool, reverseFromClaude } from './resources/agent-format.js'; import type { ToolName } from './resources/agent-format.js'; // ─── Built-in agents deployment ────────────────────────── @@ -78,8 +78,7 @@ async function removeStaleAgentSiblings(targetAgentsDir: string, stem: string, t return; // dir missing or unreadable — nothing to clean } for (const file of files) { - const base = file.replace(/\.(md|toml|json)$/, ''); - if (base !== stem) continue; + if (agentStemFromFilename(file) !== stem) continue; if (file === `${stem}${targetExt}`) continue; try { await remove(path.join(targetAgentsDir, file)); diff --git a/src/doctor.ts b/src/doctor.ts index 36724380..d3e07ca6 100644 --- a/src/doctor.ts +++ b/src/doctor.ts @@ -2,12 +2,13 @@ import path from 'node:path'; import { detectProjectConfig, loadLocalConfig, loadTeamConfig } from './config.js'; import { pathExists, readFileSafe } from './utils/fs.js'; import { log } from './utils/logger.js'; -import type { GlobalOptions, Scope } from './types.js'; +import type { GlobalOptions } from './types.js'; import { - TeamaiConfigSchema, TEAMAI_ENV_START, resolveHookScope, getDataHome, + isAgentExcluded, + scopedToolPaths, type TeamaiConfig, } from './types.js'; import { TEAMAI_HOOK_SUBCOMMANDS, isCodexTrustGatedTool, codexTrustReminder } from './hooks.js'; @@ -66,30 +67,37 @@ async function hasInstalledCodexHooks(toolPaths: TeamaiConfig['toolPaths'], base return false; } -export async function doctor(options: GlobalOptions): Promise { +export async function doctor(options: GlobalOptions): Promise { log.info('Running diagnostics...\n'); const projectConfig = await detectProjectConfig(); const localConfig = projectConfig ?? (await loadLocalConfig()); - const scope: Scope = localConfig?.scope ?? 'user'; - const configPathLabel = projectConfig - ? `${projectConfig.projectRoot}/.teamai/config.yaml` - : '~/.teamai/config.yaml'; + if (!localConfig) { + console.log(' Scope: not initialized\n'); + console.log(' ✖ TeamAI is not initialized'); + console.log(' → Run `teamai init ` in a project, or add `--scope user` for all projects'); + console.log(''); + log.warn('Initialization is required before diagnostics can run.'); + return false; + } - console.log(` Scope: ${scope}${scope === 'project' && localConfig?.projectRoot ? ` (${localConfig.projectRoot})` : ''}\n`); + const scope = localConfig.scope ?? 'user'; + const scopeLabel = `${scope}${scope === 'project' && localConfig.projectRoot ? ` (${localConfig.projectRoot})` : ''}`; + console.log(` Scope: ${scopeLabel}\n`); // Try to load team config for dynamic tool paths and provider - let teamConfig: TeamaiConfig | null = null; - if (localConfig) { - teamConfig = await loadTeamConfig(localConfig.repo.localPath); - } - // Fall back to schema defaults if team config is unavailable - const toolPaths = teamConfig?.toolPaths ?? TeamaiConfigSchema.shape.toolPaths.parse(undefined); - const providerName = teamConfig?.provider ?? 'tgit'; + const teamConfig = await loadTeamConfig(localConfig.repo.localPath); + const toolPaths: TeamaiConfig['toolPaths'] = teamConfig + ? Object.fromEntries( + Object.entries(scopedToolPaths(teamConfig, localConfig)) + .filter(([tool]) => !isAgentExcluded(localConfig, tool)), + ) + : {}; + const providerName = teamConfig?.provider; // Hook checks must look where hooks are actually injected. resolveHookScope // maps a non-self project scope to HOME (#264), matching the injection path in // init/pull/hooks-cmd — otherwise doctor checks /.claude while the // hooks live in ~/.claude and always reports them missing. - const baseDir = localConfig ? resolveHookScope(localConfig).baseDir : getUserHome(); + const baseDir = resolveHookScope(localConfig).baseDir; const checks: Check[] = []; @@ -145,23 +153,14 @@ export async function doctor(options: GlobalOptions): Promise { } checks.push( - { - name: `Local config exists (${configPathLabel})`, - check: async () => localConfig !== null, - fix: 'Run `teamai init` to initialize', - }, { name: 'Team repo exists locally', - check: async () => { - if (!localConfig) return false; - return pathExists(localConfig.repo.localPath); - }, + check: async () => pathExists(localConfig.repo.localPath), fix: 'Run `teamai init` to clone the team repo', }, { name: 'Team config (teamai.yaml) is valid', check: async () => { - if (!localConfig) return false; const config = await loadTeamConfig(localConfig.repo.localPath); return config !== null; }, @@ -173,7 +172,6 @@ export async function doctor(options: GlobalOptions): Promise { check: async () => { if (teamConfig?.sharing?.env?.injectShellProfile === false) return true; - if (!localConfig) return true; const envYamlPath = path.join(localConfig.repo.localPath, 'env', 'env.yaml'); if (!await pathExists(envYamlPath)) return true; @@ -212,13 +210,11 @@ export async function doctor(options: GlobalOptions): Promise { } } - if (localConfig) { - const { pkgDoctorReport } = await import('./pkg/commands.js'); - const packageReport = await pkgDoctorReport(localConfig, process.cwd()); - if (packageReport) { - for (const line of packageReport.lines) console.log(line); - if (!packageReport.allPassed) allPassed = false; - } + const { pkgDoctorReport } = await import('./pkg/commands.js'); + const packageReport = await pkgDoctorReport(localConfig, process.cwd()); + if (packageReport) { + for (const line of packageReport.lines) console.log(line); + if (!packageReport.allPassed) allPassed = false; } // Codex trust-gate reminder: even when hooks are installed, Codex may not run @@ -235,4 +231,5 @@ export async function doctor(options: GlobalOptions): Promise { } else { log.warn('Some checks failed. See suggestions above.'); } + return allPassed; } diff --git a/src/index.ts b/src/index.ts index 7fa70798..8ea8e744 100644 --- a/src/index.ts +++ b/src/index.ts @@ -218,7 +218,8 @@ program .action(async () => { const globalOpts = program.opts() as GlobalOptions; const { doctor } = await import('./doctor.js'); - await doctor(globalOpts); + const allPassed = await doctor(globalOpts); + if (!allPassed) process.exitCode = 1; }); // ─── Roles subcommand ───────────────────────────────────── diff --git a/src/init.ts b/src/init.ts index a0a73f7d..529b2103 100644 --- a/src/init.ts +++ b/src/init.ts @@ -476,6 +476,23 @@ export async function initHttp( log.debug(`Local agent init: ${(e as Error).message}`); } + // Step 6.5: init --agent explicitly declares which tools to set up, so + // seed their dirs and do one real sync now instead of waiting for a + // SessionStart hook that may never fire (issue #574/#585). + if (requestedAgents.length > 0) { + try { + const { seedEnabledAgentDirs } = await import('./known-agents.js'); + const seeded = await seedEnabledAgentDirs(localConfig, teamConfig); + if (seeded.length > 0) { + const { pullForScope } = await import('./pull.js'); + await pullForScope(localConfig, {}); + log.success(`Synced skills, rules, and docs for: ${seeded.join(', ')}`); + } + } catch (e) { + log.warn(`Initial sync skipped: ${(e as Error).message}`); + } + } + log.success('teamai initialized (HTTP read-only)!'); log.info('Skills/rules will auto-sync on each session start via report/sync. This team is read-only (no push).'); closePrompt(); @@ -883,7 +900,7 @@ export async function initSelfRepo(options: GlobalOptions & { // Which AI tools to set up in this repo (create skills dir + inject hooks + // commit their settings.json). Resolved from --agent, else HOME detection // (non-interactive), else an interactive picker. Written to enabledAgents, - // which drives seedSelfModeToolDirs and hook injection alike. + // which drives seedEnabledAgentDirs and hook injection alike. const selectedAgents = await promptForSelfModeAgents(options); if (selectedAgents.length > 0) { const existing = await loadLocalConfigForScope('project', businessRepoRoot); @@ -914,8 +931,8 @@ export async function initSelfRepo(options: GlobalOptions & { // otherwise skip everything). const filterAgents = selectedAgents.length > 0 ? selectedAgents : undefined; try { - const { seedSelfModeToolDirs } = await import('./known-agents.js'); - const seeded = await seedSelfModeToolDirs(localConfig, teamConfig); + const { seedEnabledAgentDirs } = await import('./known-agents.js'); + const seeded = await seedEnabledAgentDirs(localConfig, teamConfig); if (seeded.length > 0) log.debug(`Seeded tool dirs for: ${seeded.join(', ')}`); } catch (e) { log.debug(`Tool-dir seeding skipped: ${(e as Error).message}`); @@ -1624,6 +1641,23 @@ export async function init(options: GlobalOptions & { } catch (e) { log.debug(`Built-in skills deployment skipped: ${(e as Error).message}`); } + + // Step 7.6: init --agent explicitly declares which tools to set up, so + // seed their dirs and do one real sync now instead of waiting for a + // SessionStart hook that may never fire (issue #574/#585). + if (requestedAgents.length > 0) { + try { + const { seedEnabledAgentDirs } = await import('./known-agents.js'); + const seeded = await seedEnabledAgentDirs(localConfig, reloadedTeamConfig); + if (seeded.length > 0) { + const { pullForScope } = await import('./pull.js'); + await pullForScope(localConfig, {}); + log.success(`Synced skills, rules, and docs for: ${seeded.join(', ')}`); + } + } catch (e) { + log.warn(`Initial sync skipped: ${(e as Error).message}`); + } + } } log.success('teamai initialized successfully!'); diff --git a/src/known-agents.ts b/src/known-agents.ts index cc6c01d6..b33b962a 100644 --- a/src/known-agents.ts +++ b/src/known-agents.ts @@ -150,7 +150,7 @@ export interface ResolvedAgent extends KnownAgent { * * Returns the list of agent ids whose dirs were ensured. */ -export async function seedSelfModeToolDirs( +export async function seedEnabledAgentDirs( localConfig: LocalConfig, teamConfig: TeamaiConfig, ): Promise { diff --git a/src/pull.ts b/src/pull.ts index f56ab2ac..fa0153d1 100644 --- a/src/pull.ts +++ b/src/pull.ts @@ -9,6 +9,7 @@ import { injectClaudeMdSection } from './utils/claudemd.js'; import { getHandler, RulesHandler, DocsHandler, EnvHandler, AgentsHandler } from './resources/index.js'; import { ResourceHandler } from './resources/base.js'; import { ruleFileExtensionForTool } from './resources/rule-format.js'; +import { AGENT_FILE_EXTENSIONS } from './resources/agent-format.js'; import { loadTagsConfig, filterByTags } from './utils/tags.js'; import { BUILTIN_SKILL_NAMES } from './builtin-skills.js'; import type { GlobalOptions, ResourceType, ResourceItem, TeamaiConfig, LocalConfig, TagsConfig } from './types.js'; @@ -471,34 +472,121 @@ function logSyncDetail( * this set alongside the revision prevents a pull for one tool from suppressing * the first resource sync for another. */ -async function getInstalledResourceTargets( +/** + * Tool ids whose directory currently exists for this scope, excluding + * agents disabled via enabledAgents/disabledAgents. The same walk (installed + * dir → not excluded) previously repeated once per resource type; shared here + * so rules/skills/agents' "did anything actually get written" checks and the + * revision-cache target list all agree on one answer (issue #574/#585). + */ +async function installedToolsFor( teamConfig: TeamaiConfig, localConfig: LocalConfig, + field: 'rules' | 'skills' | 'agents', ): Promise { const baseDir = resolveBaseDir(localConfig); - const targets: string[] = []; - + const found: string[] = []; for (const [tool, toolPath] of Object.entries(scopedToolPaths(teamConfig, localConfig))) { if (isAgentExcluded(localConfig, tool)) continue; + const dir = toolPath[field]; + if (!dir) continue; + if (await ResourceHandler.isToolInstalled(dir, baseDir)) found.push(tool); + } + return found; +} - const resourcePaths = [toolPath.skills, toolPath.rules, toolPath.agents] - .filter((resourcePath): resourcePath is string => !!resourcePath); - for (const resourcePath of resourcePaths) { - if (await ResourceHandler.isToolInstalled(resourcePath, baseDir)) { - targets.push(tool); - break; - } +/** Whether at least one configured tool's directory currently exists. */ +async function hasInstalledTargetFor( + teamConfig: TeamaiConfig, + localConfig: LocalConfig, + field: 'rules' | 'skills' | 'agents', +): Promise { + return (await installedToolsFor(teamConfig, localConfig, field)).length > 0; +} + +async function getInstalledResourceTargets( + teamConfig: TeamaiConfig, + localConfig: LocalConfig, +): Promise { + const targets = new Set(); + for (const field of ['skills', 'rules', 'agents'] as const) { + for (const tool of await installedToolsFor(teamConfig, localConfig, field)) { + targets.add(tool); } } + return [...targets].sort(); +} - return targets.sort(); +/** + * Every extension a tombstoned resource may wear in a tool's directory. + * + * Rules carry a per-tool extension (`.mdc` for compatible tools), and those + * dirs may still hold a `.md` copy from the layout that predates it. Agents are + * rendered per tool as `.md`, `.toml` or `.json`. Skills are directories, so + * their empty suffix leaves the bare name. + */ +function tombstoneExtensions(type: ResourceType, tool: string): readonly string[] { + if (type === 'rules') return [...new Set([ruleFileExtensionForTool(tool), '.md'])]; + if (type === 'agents') return AGENT_FILE_EXTENSIONS; + return ['']; +} + +/** + * Delete the local copies of every resource the team has tombstoned. + * + * Called from the full sync and from the "already synced" fast path: a CLI + * upgrade that widens the extensions above must still reach a machine whose + * team repo HEAD has not moved since it pulled the tombstone (issue #576). + */ +async function cleanupTombstonedResources( + freshConfig: TeamaiConfig, + localConfig: LocalConfig, + scopeLabel: string, +): Promise { + // Each entry maps a resource type to the field on toolPath that names the + // tool-side directory; `tombstoneExtensions` supplies the filename suffixes. + const tombstoneTypes: { type: ResourceType; toolPathField: 'rules' | 'skills' | 'agents' }[] = [ + { type: 'rules', toolPathField: 'rules' }, + { type: 'skills', toolPathField: 'skills' }, + { type: 'agents', toolPathField: 'agents' }, + ]; + + const baseDir = resolveBaseDir(localConfig); + for (const { type, toolPathField } of tombstoneTypes) { + const handler = getHandler(type); + const tombstones = await handler.readTombstones(localConfig); + if (tombstones.size === 0) continue; + + for (const [tool, toolPath] of Object.entries(scopedToolPaths(freshConfig, localConfig))) { + const dir = toolPath[toolPathField]; + if (!dir) continue; + if (!await ResourceHandler.isToolInstalled(dir, baseDir)) continue; + if (isAgentExcluded(localConfig, tool)) continue; + + for (const name of tombstones) { + for (const extension of tombstoneExtensions(type, tool)) { + const localPath = path.join(baseDir, dir, `${name}${extension}`); + if (!await pathExists(localPath)) continue; + // Even an upstream (tombstone) removal must not blow away a local + // repo's stash/unpushed history inside a skill directory. Keep + // + warn; the user can delete it manually once backed up. + if (type === 'skills' && await hasVcsMetadataRecursive(localPath)) { + log.warn(`[${scopeLabel}] Kept tombstoned skill "${name}" (${tool}): it has local VCS metadata (.git) that may hold unpushed history. Back it up, then delete it manually.`); + continue; + } + await remove(localPath); + log.debug(`[${scopeLabel}] Cleaned up tombstoned ${type} ${name} from ${dir}`); + } + } + } + } } /** * Pull resources for a single scope. This is the core sync logic extracted * from the original pull() function to support both user and project scope. */ -async function pullForScope( +export async function pullForScope( localConfig: LocalConfig, options: GlobalOptions, policy: { @@ -568,6 +656,10 @@ async function pullForScope( // Also refresh the CLAUDE.md recall block so a CLI upgrade that ships // a new block reaches CLAUDE.md even when the repo HEAD is unchanged. await injectRecallBlockIntoTools(freshConfig, localConfig, scopeLabel); + // Same reason: a machine that already pulled a tombstone with an older + // CLI keeps the copies that CLI failed to delete, and its stored rev + // never moves again. Re-run the cleanup so the upgrade reaches it (#576). + await cleanupTombstonedResources(freshConfig, localConfig, scopeLabel); return; } @@ -621,12 +713,17 @@ async function pullForScope( // stale local rule files and deactivates the OpenCode instructions glob // when the team's last rule is removed. Guarding on items.length > 0 // would leak those artifacts on the machine after upstream deletion. - await rulesHandler.pullAllRules(freshConfig, localConfig, items); + await rulesHandler.pullAllRules(freshConfig, localConfig, items, options.force); if (items.length > 0) { - log.success(`[${scopeLabel}] Synced ${items.length} rule(s)${skipped.length > 0 ? ` (skipped ${skipped.length} by tags)` : ''}`); + const hasTarget = await hasInstalledTargetFor(freshConfig, localConfig, 'rules'); + if (hasTarget) { + log.success(`[${scopeLabel}] Synced ${items.length} rule(s)${skipped.length > 0 ? ` (skipped ${skipped.length} by tags)` : ''}`); + totalSynced += items.length; + } else { + log.warn(`[${scopeLabel}] ${items.length} rule(s) available but no installed tool directory found — nothing written. Create the tool's directory (e.g. mkdir .claude) and pull again, or use teamai init --agent.`); + } } } - totalSynced += items.length; continue; } @@ -730,72 +827,27 @@ async function pullForScope( } } } else { - for (const item of items) { - await handler.pullItem(item, freshConfig, localConfig); - } - - if (type === 'skills') { - logSyncDetail(type, items, existingNames, !!options.verbose, scopeLabel, skippedByTags); + const installed = await installedToolsFor(freshConfig, localConfig, type as 'skills' | 'agents'); + if (installed.length === 0) { + log.warn(`[${scopeLabel}] ${items.length} ${type} available but no installed tool directory found — nothing written. Create the tool's directory and pull again, or use teamai init --agent.`); } else { - log.success(`[${scopeLabel}] Synced ${items.length} ${type}`); + for (const item of items) { + await handler.pullItem(item, freshConfig, localConfig); + } + + if (type === 'skills') { + logSyncDetail(type, items, existingNames, !!options.verbose, scopeLabel, skippedByTags); + } else { + log.success(`[${scopeLabel}] Synced ${items.length} ${type} → ${installed.join(', ')}`); + } + totalSynced += items.length; } } - - totalSynced += items.length; } // Step 3: Clean up tombstoned resources if (!options.dryRun) { - // Each entry maps a resource type to (a) the field on toolPath that names - // the tool-side directory and (b) the filename suffix used for that - // resource on disk (e.g. rules/wiki pages are files, skills are dirs). - const tombstoneTypes: { - type: ResourceType; - ext?: string; - toolPathField: 'rules' | 'skills' | 'agents'; - }[] = [ - { type: 'rules', ext: '.md', toolPathField: 'rules' }, - { type: 'skills', toolPathField: 'skills' }, - { type: 'agents', ext: '.md', toolPathField: 'agents' }, - ]; - - const baseDir = resolveBaseDir(localConfig); - for (const { type, ext, toolPathField } of tombstoneTypes) { - const handler = getHandler(type); - const tombstones = await handler.readTombstones(localConfig); - if (tombstones.size === 0) continue; - - for (const [tool, toolPath] of Object.entries(scopedToolPaths(freshConfig, localConfig))) { - const dir = toolPath[toolPathField]; - if (!dir) continue; - if (!await ResourceHandler.isToolInstalled(dir, baseDir)) continue; - if (isAgentExcluded(localConfig, tool)) continue; - - // Rules carry a per-tool extension (`.mdc` for compatible tools), and those dirs - // may still hold a `.md` copy from the layout that predates it, so a - // tombstoned rule is cleaned up under every extension it may wear. - const extensions = type === 'rules' - ? [...new Set([ruleFileExtensionForTool(tool), '.md'])] - : [ext]; - - for (const name of tombstones) { - for (const extension of extensions) { - const localPath = path.join(baseDir, dir, extension ? `${name}${extension}` : name); - if (await pathExists(localPath)) { - // Even an upstream (tombstone) removal must not blow away a local - // git repo's stash/unpushed history inside a skill directory. Keep - // + warn; the user can delete it manually once backed up. - if (type === 'skills' && await hasVcsMetadataRecursive(localPath)) { - log.warn(`[${scopeLabel}] Kept tombstoned skill "${name}" (${tool}): it has local VCS metadata (.git) that may hold unpushed history. Back it up, then delete it manually.`); - continue; - } - await remove(localPath); - log.debug(`[${scopeLabel}] Cleaned up tombstoned ${type} ${name} from ${dir}`); - } - } - } - } - } + await cleanupTombstonedResources(freshConfig, localConfig, scopeLabel); if (roleContext) { await cleanupInactiveNamespaceSkills( diff --git a/src/resources/agent-format.ts b/src/resources/agent-format.ts index 9f156ed4..46e81968 100644 --- a/src/resources/agent-format.ts +++ b/src/resources/agent-format.ts @@ -26,6 +26,25 @@ export const ALL_SUPPORTED_TOOLS: ToolName[] = [ export type AgentFileExtension = '.md' | '.toml' | '.json'; +/** + * Every extension an agent render may carry on disk. + * + * Writers use `agentFileExtensionForTool`. Scanners and deleters use this list, + * so a removal clears a name on every tool whatever format that tool renders. + */ +export const AGENT_FILE_EXTENSIONS = ['.md', '.toml', '.json'] as const satisfies readonly AgentFileExtension[]; + +/** + * Extract an agent name stem from a filename. + * Accepts every native agent extension; returns null for other files. + */ +export function agentStemFromFilename(filename: string): string | null { + for (const ext of AGENT_FILE_EXTENSIONS) { + if (filename.endsWith(ext)) return filename.slice(0, -ext.length); + } + return null; +} + export function agentFileExtensionForTool(tool: ToolName): AgentFileExtension { switch (tool) { case 'codex': diff --git a/src/resources/agents.ts b/src/resources/agents.ts index db4c4fe9..a7be3979 100644 --- a/src/resources/agents.ts +++ b/src/resources/agents.ts @@ -23,6 +23,8 @@ import { reverseFromOpencode, mergeReverseResults, ALL_SUPPORTED_TOOLS, + AGENT_FILE_EXTENSIONS, + agentStemFromFilename, } from './agent-format.js'; import type { AgentSpec, ToolName, ReverseResult, ParseResult, MergeResult, RenderResult } from './agent-format.js'; @@ -114,7 +116,7 @@ export class AgentsHandler extends ResourceHandler { const files = await listFiles(agentsDir); for (const file of files) { - const stem = getAgentStem(file); + const stem = agentStemFromFilename(file); if (stem === null) continue; if (tombstones.has(stem)) continue; if (BUILTIN_AGENT_NAMES.has(stem)) continue; @@ -457,8 +459,11 @@ export class AgentsHandler extends ResourceHandler { for (const [tool, toolPath] of Object.entries(scopedToolPaths(teamConfig, localConfig))) { if (!toolPath.agents) continue; - // Try every native agent extension. - for (const ext of ['.md', '.toml', '.json'] as const) { + // A tool the member excluded is not ours to write to, so it is not ours + // to delete from either. This is the gate pull's tombstone pass applies. + if (isAgentExcluded(localConfig, tool)) continue; + // Try every native agent extension: the render format varies per tool. + for (const ext of AGENT_FILE_EXTENSIONS) { const filePath = path.join(baseDir, toolPath.agents, `${name}${ext}`); if (await pathExists(filePath)) { await remove(filePath); @@ -661,21 +666,10 @@ function mergeCanonicalEdits( return { ok: true, spec: merged }; } -/** - * Extract agent name stem from a filename. - * Accepts supported native agent extensions; returns null for other files. - */ -function getAgentStem(filename: string): string | null { - if (filename.endsWith('.md')) return filename.slice(0, -3); - if (filename.endsWith('.toml')) return filename.slice(0, -5); - if (filename.endsWith('.json')) return filename.slice(0, -5); - return null; -} - /** Remove an obsolete same-stem native rendering after a format migration. */ async function removeStaleAgentSiblings(agentsDir: string, stem: string, targetExt: string): Promise { for (const file of await listFiles(agentsDir)) { - if (getAgentStem(file) !== stem || file === `${stem}${targetExt}`) continue; + if (agentStemFromFilename(file) !== stem || file === `${stem}${targetExt}`) continue; await remove(path.join(agentsDir, file)); log.debug(`Removed stale agent sibling ${file} for ${stem}`); } diff --git a/src/resources/rules.ts b/src/resources/rules.ts index 077941f2..13e1e97a 100644 --- a/src/resources/rules.ts +++ b/src/resources/rules.ts @@ -221,6 +221,9 @@ export class RulesHandler extends ResourceHandler { // would report success while leaving the rule on disk. for (const [tool, toolPath] of Object.entries(scopedToolPaths(teamConfig, localConfig))) { if (!toolPath.rules) continue; + // Not ours to write to, so not ours to delete from. Same gate as the + // tombstone pass in pull. + if (isAgentExcluded(localConfig, tool)) continue; const extensions = new Set([ruleFileExtensionForTool(tool), '.md']); for (const extension of extensions) { const filePath = path.join(baseDir, toolPath.rules, `${name}${extension}`); @@ -246,6 +249,7 @@ export class RulesHandler extends ResourceHandler { teamConfig: TeamaiConfig, localConfig: LocalConfig, filteredRules?: ResourceItem[], + force?: boolean, ): Promise { const rules = filteredRules ?? await this.scanTeamForPull(teamConfig, localConfig); @@ -280,6 +284,13 @@ export class RulesHandler extends ResourceHandler { if (rules.length === 0) return; // 1. Distribute rule files to each tool's rules/ directory + if (force) { + const baseDir = resolveBaseDir(localConfig); + for (const [, toolPath] of Object.entries(scopedToolPaths(teamConfig, localConfig))) { + if (!toolPath.rules) continue; + await ensureDir(path.join(baseDir, toolPath.rules)); + } + } for (const rule of rules) { await this.pullItem(rule, teamConfig, localConfig); } @@ -290,6 +301,9 @@ export class RulesHandler extends ResourceHandler { const baseDir = resolveBaseDir(localConfig); for (const [tool, toolPath] of Object.entries(scopedToolPaths(teamConfig, localConfig))) { if (!toolPath.rules) continue; + // `pullItem` above skips excluded tools, so this pass must skip them too. + // Without it the stale sweep deletes from a directory teamai never wrote. + if (isAgentExcluded(localConfig, tool)) continue; if (!await ResourceHandler.isToolInstalled(toolPath.rules, baseDir)) continue; const destDir = path.join(baseDir, toolPath.rules); diff --git a/src/resources/skills.ts b/src/resources/skills.ts index 8b56ccc8..28eb1ab6 100644 --- a/src/resources/skills.ts +++ b/src/resources/skills.ts @@ -550,6 +550,9 @@ export class SkillsHandler extends ResourceHandler { // Remove from each tool's skills directory for (const [tool, toolPath] of Object.entries(scopedToolPaths(teamConfig, localConfig))) { if (!toolPath.skills) continue; + // Not ours to write to, so not ours to delete from. Above the OpenClaw + // branch, so the workspace copy is covered by the same gate. + if (isAgentExcluded(localConfig, tool)) continue; let skillDir: string; if (tool === 'openclaw') { const wsDir = await resolveOpenclawWorkspaceDir(); diff --git a/src/uninstall.ts b/src/uninstall.ts index 6b9376d3..4ffb5599 100644 --- a/src/uninstall.ts +++ b/src/uninstall.ts @@ -32,6 +32,7 @@ import { } from './types.js'; import { BUILTIN_RULE_NAMES } from './builtin-rules.js'; import { ruleStemFromFilename } from './resources/rule-format.js'; +import { agentStemFromFilename } from './resources/agent-format.js'; import { listTeamAgentDirs } from './resources/agents.js'; import { BUILTIN_AGENT_NAMES } from './builtin-agents.js'; import { BUILTIN_SKILL_NAMES } from './builtin-skills.js'; @@ -338,8 +339,8 @@ async function discoverToolResources( const agentsDir = path.join(baseDir, toolPath.agents); if (await pathExists(agentsDir)) { for (const file of await listFiles(agentsDir)) { - if (!file.endsWith('.md') && !file.endsWith('.toml') && !file.endsWith('.json')) continue; - const name = path.basename(file).replace(/\.(md|toml|json)$/, ''); + const name = agentStemFromFilename(path.basename(file)); + if (name === null) continue; if (!teamAgentNames.has(name) && !BUILTIN_AGENT_NAMES.has(name)) continue; res.agentFiles.push(path.join(agentsDir, file)); }