diff --git a/docs/designs/skill-serving.md b/docs/designs/skill-serving.md index f2494994..a20dd5f4 100644 --- a/docs/designs/skill-serving.md +++ b/docs/designs/skill-serving.md @@ -94,14 +94,17 @@ path (measured here from a 77-character one). blocks instead (`blockedBy: "config"`), since recall and the source are then unknown and the workflow would fail at `teamai contribute` — a project config too, which detection alone would skip in favour of the user config - (`findUnreadableProjectConfig`). The Stop-hook share reminder is gated the same - way (`contributeHintAllowed`, `src/hook-handlers.ts`), because it points at this - command, with one difference: with no config at all it stays silent. The hook - fires in every project on the machine, and a directory without teamai has no - team to share with (#748). The gate lives in one place: - `resolveServableSkill` (`src/skill-content.ts`) is the only way to obtain a - packaged skill outside that module, and it returns `blocked` instead of the - skill, so a command cannot print a directory it never received. + (`findUnreadableProjectConfig`), including one that is not `scope: project`. The refusal then says what failed (for a file + that does not parse, which file and where; for one that fails validation, + which field and why), since nothing else reports it. The + Stop-hook share reminder asks the same gate (`contributeHintAllowed`, called + by the hook dispatcher and by the legacy `teamai contribute-check`), because it + points at this command, with one difference: with no config at all it stays + silent. The hook fires in every project on the machine, and a directory without + teamai has no team to share with (#748). The gate lives in one place: `shareGate` + (`src/skill-content.ts`) decides it, and `resolveServableSkill` is the only + way to obtain a packaged skill outside that module; it returns `blocked` + instead of the skill, so a command cannot print a directory it never received. - **`skill path` takes a name, always,** and a blocked name gets the same refusal as `skill get`. The gate routes the agent away from a workflow that cannot finish; it is not access control, since the files ship in the package. @@ -109,7 +112,10 @@ path (measured here from a 77-character one). team repo, then the installed agents, then the package. `codebase`, `default`, `learning` and `share` are ordinary names: a directory a member created under one of them is the skill they are asking about, and the recall gate does not - apply to it. The two legacy directory names are the exception, by design: + apply to it. A config that cannot be loaded does apply: which team repo and + agents are meant is then unknown (`detectTeam`), so `skill show` searches + neither, refuses `share`, and answers any other name from the package alone, + saying what failed. The two legacy directory names are the exception, by design: `team-wiki-codebase` and `teamai-share-learnings` classify as `[builtin]` and are skipped by the push scan by name alone (`isCliOwnedSkillName`), because a tree with that name is one a pre-stub release wrote until the first pull has @@ -121,6 +127,8 @@ path (measured here from a 77-character one). - **`skill list` needs no team.** The human-readable listing prints the packaged catalog even before `teamai init`, with a hint for the team half, so a fresh machine can discover what the installed CLI serves the way `skill get` lets it. + With a config that cannot be loaded it prints the catalog too, but no team + listing, says what failed on stderr, and exits 1. ## Drift guards @@ -132,8 +140,9 @@ Two tests, both in the unit suite: `npx vitest run commands-reference -u`. - `skill-commands-exist.test.ts` resolves every `teamai …` string written anywhere under `skill-data/` against that same table, and fails on an unknown - command or flag. It carries a case proving it catches `teamai extract graph`, - the command the wiki skill advertised for four releases. + command, subcommand or flag. It carries cases proving it catches + `teamai extract graph`, the command the wiki skill advertised for four + releases, and a misspelled subcommand inside a group (`teamai skill gett`). A third, in `skill-content.test.ts`, asserts through `npm pack` that both `skills/` and `skill-data/` are in the published tarball. Without it, a missing @@ -237,7 +246,8 @@ and can be dropped on the same schedule. `/teamai-share-learnings` was never a deployed slash command in its own right — it existed because the directory was installed. The Stop-hook nudge now names `/teamai share what this session taught me`, an invocation the core skill routes -to `share` (bare `/teamai` prints the menu and stops), and carries +to `share` (so does a bare `/teamai` typed right after the reminder; otherwise +bare `/teamai` prints the menu and stops), and carries `teamai skill get share` literally, so an agent can act on it even without -inferring the intent. It is withheld while recall is off, because that command -refuses then. +inferring the intent. It is withheld wherever that command refuses (recall off, +a read-only HTTP source, a config that cannot be loaded). diff --git a/docs/product-overview.md b/docs/product-overview.md index a1c9d8b8..7d42b1bf 100644 --- a/docs/product-overview.md +++ b/docs/product-overview.md @@ -121,7 +121,7 @@ Task: Fix duplicate project-level Hook injection Consider running `/teamai share what this session taught me` to summarize what you learned and share it with your team (or run `teamai skill get share`). ``` -The hint names the non-zero friction signals that triggered it and, when available, includes a redacted, single-line summary of the first task. The `share` workflow (`teamai skill get share`) summarizes the session and pushes a learning document directly to the team repo. Each session is prompted at most once. Teams can switch the hint off with `sharing.contributeHint.enabled: false` in `teamai.yaml` (members: `contributeHintEnabled` in local config) while keeping the rest of the Stop hook. The hint also needs recall to be on (it is off by default), because the workflow it points at is served only then, and it never appears in a directory where teamai is not set up. +The hint names the non-zero friction signals that triggered it and, when available, includes a redacted, single-line summary of the first task. The `share` workflow (`teamai skill get share`) summarizes the session and pushes a learning document directly to the team repo. Each session is prompted at most once. Teams can switch the hint off with `sharing.contributeHint.enabled: false` in `teamai.yaml` (members: `contributeHintEnabled` in local config) while keeping the rest of the Stop hook. The hint also needs recall to be on (it is off by default), because the workflow it points at is served only then. For the same reason it never appears on a read-only HTTP source or while a teamai config exists but cannot be loaded, and it never appears in a directory where teamai is not set up. ### Team Knowledge Recall diff --git a/docs/product-overview.zh-CN.md b/docs/product-overview.zh-CN.md index 0294c9fa..9accc515 100644 --- a/docs/product-overview.zh-CN.md +++ b/docs/product-overview.zh-CN.md @@ -121,7 +121,7 @@ Task: Fix duplicate project-level Hook injection Consider running `/teamai share what this session taught me` to summarize what you learned and share it with your team (or run `teamai skill get share`). ``` -提示会列出实际触发它的非零摩擦信号；如果能取得首个任务摘要，还会在脱敏、单行化后附上任务上下文。`share` 工作流（`teamai skill get share`）自动总结 session 经验并推送到团队仓库。每个 session 最多提示一次。团队可在 `teamai.yaml` 设置 `sharing.contributeHint.enabled: false` 关闭该提示（成员可用本地配置 `contributeHintEnabled` 覆盖），Stop hook 的其余功能不受影响。该提示还需要开启 recall（默认关闭），因为它指向的工作流只在 recall 开启时提供；在未配置 teamai 的目录中也不会出现。 +提示会列出实际触发它的非零摩擦信号；如果能取得首个任务摘要，还会在脱敏、单行化后附上任务上下文。`share` 工作流（`teamai skill get share`）自动总结 session 经验并推送到团队仓库。每个 session 最多提示一次。团队可在 `teamai.yaml` 设置 `sharing.contributeHint.enabled: false` 关闭该提示（成员可用本地配置 `contributeHintEnabled` 覆盖），Stop hook 的其余功能不受影响。该提示还需要开启 recall（默认关闭），因为它指向的工作流只在 recall 开启时提供。同理，只读 HTTP 源上，或 teamai 配置文件存在但无法加载时，该提示从不出现；在未配置 teamai 的目录中也不会出现。 ### 团队知识检索 diff --git a/docs/usage-guide.md b/docs/usage-guide.md index 9b421036..58f339ae 100644 --- a/docs/usage-guide.md +++ b/docs/usage-guide.md @@ -507,7 +507,9 @@ yours and stays. A directory that also holds a file of your own is kept, with on packaged files removed, and named in the pull output. `share` is served only while recall is on (off by default; `sharing.recall.enabled: true` in `teamai.yaml` for the team, or `teamai recall enable` for one machine): until then `teamai skill get share` refuses and says so. -It also refuses on a read-only HTTP source, where `teamai contribute` cannot write. The legacy names still +It also refuses on a read-only HTTP source, where `teamai contribute` cannot write, and when a +teamai config exists but cannot be loaded (the refusal says what failed: for a file that does not parse, which file and where; for one that fails validation, which field and why), +since recall and the source are then unknown. The legacy names still resolve: `teamai skill get team-wiki-codebase` serves `wiki`. --- @@ -973,7 +975,7 @@ Teams that route knowledge sharing through their own review flow (for example, a Only the nudge is affected: friction scoring, `teamai contribute --file`, and `/teamai` keep working when invoked manually. -The reminder is also withheld while recall is off (the default until `sharing.recall.enabled: true` in `teamai.yaml`, or `teamai recall enable` on one machine): it points at the `share` workflow, and `teamai skill get share` refuses until recall is on. It never appears on a read-only HTTP source, where `share` refuses too, nor in a directory where teamai is not set up. +The reminder is also withheld while recall is off (the default until `sharing.recall.enabled: true` in `teamai.yaml`, or `teamai recall enable` on one machine): it points at the `share` workflow, and `teamai skill get share` refuses until recall is on. It never appears on a read-only HTTP source, or while a teamai config exists but cannot be loaded, where `share` refuses too, nor in a directory where teamai is not set up, although `teamai skill get share` still serves there. ### Searching knowledge diff --git a/docs/usage-guide.zh-CN.md b/docs/usage-guide.zh-CN.md index 7d8666d9..218aeb2e 100644 --- a/docs/usage-guide.zh-CN.md +++ b/docs/usage-guide.zh-CN.md @@ -473,7 +473,8 @@ teamai skill path wiki # 打印打包目录，用于运行 skill 只删除其中的打包文件，保留该目录和你的文件，并在 pull 输出中点名。`share` 只在开启 recall 后才会提供（默认关闭； 团队在 `teamai.yaml` 设置 `sharing.recall.enabled: true`，或单台机器运行 `teamai recall enable`）：在此之前， `teamai skill get share` 会拒绝并说明原因。 -只读 HTTP 源上它同样会拒绝，因为 `teamai contribute` 无法写入。旧名字仍然可用： +只读 HTTP 源上它同样会拒绝，因为 `teamai contribute` 无法写入；teamai 配置文件存在但无法加载时也会拒绝 +（提示会说明失败原因；若是文件无法解析，还会指出是哪个文件、哪一行；若是校验失败，还会指出是哪个字段、为何不合法），因为此时无法确定 recall 与来源。旧名字仍然可用： `teamai skill get team-wiki-codebase` 等价于 `wiki`。 --- @@ -934,7 +935,7 @@ teamai contribute --file /tmp/session.md --scope project 只影响提醒本身：摩擦评分、`teamai contribute --file` 和手动调用 `/teamai` 不受影响。 -未开启 recall 时（默认关闭；团队在 `teamai.yaml` 设置 `sharing.recall.enabled: true`，或单台机器运行 `teamai recall enable`）也不会显示这条提醒：提醒指向 `share` 工作流，而 recall 关闭时 `teamai skill get share` 会拒绝执行。只读 HTTP 源上这条提醒也从不出现，因为 `share` 同样会拒绝；在未配置 teamai 的目录中也不会出现。 +未开启 recall 时（默认关闭；团队在 `teamai.yaml` 设置 `sharing.recall.enabled: true`，或单台机器运行 `teamai recall enable`）也不会显示这条提醒：提醒指向 `share` 工作流，而 recall 关闭时 `teamai skill get share` 会拒绝执行。只读 HTTP 源上，或 teamai 配置文件存在但无法加载时，这条提醒也从不出现，因为 `share` 同样会拒绝；在未配置 teamai 的目录中也不会出现，尽管 `teamai skill get share` 在那里仍会提供。 ### 搜索知识 diff --git a/skill-data/core/SKILL.md b/skill-data/core/SKILL.md index b9a02ca6..c81bcf58 100644 --- a/skill-data/core/SKILL.md +++ b/skill-data/core/SKILL.md @@ -15,7 +15,11 @@ do not skip, reorder, or invent commands. Look at what the user typed after `/teamai`. -**If they gave NO scenario** (bare `/teamai`, or only greetings/no task): +**If they gave NO scenario right after a TeamAI friction reminder** (the +`[teamai]` line that suggests `/teamai share what this session taught me`), +that reminder is the scenario: load `teamai skill get share` and follow it. + +**If they gave NO scenario otherwise** (bare `/teamai`, or only greetings/no task): print the menu below **exactly**, then **STOP and wait**. Take no other action — do not run any command, do not load another skill yet. @@ -62,7 +66,8 @@ Sharing a session's learnings needs no menu choice: TeamAI prompts on its own at the end of a session that produced something worth sharing, and that prompt means `teamai skill get share`. (Only when recall is on; it is off by default. The team turns it on with `sharing.recall.enabled: true` in `teamai.yaml`, a member with `teamai recall enable`; -while it is off, `teamai skill get share` says so.) +while it is off, or while the teamai config cannot be loaded, `teamai skill get share` +says so and why.) ## Global rules diff --git a/skill-data/core/references/contribute-member.md b/skill-data/core/references/contribute-member.md index 73590fde..5b7f8481 100644 --- a/skill-data/core/references/contribute-member.md +++ b/skill-data/core/references/contribute-member.md @@ -8,7 +8,7 @@ team"*, in whatever language they work in — then you run the publish for them. ## Which kind of contribution? - **A learning** (a lesson, a gotcha, how you solved something) → this is - **automatic** once recall is on (off by default): TeamAI prompts at the end of a session worth sharing and the + **automatic** once recall is on (off by default) and the teamai config loads: TeamAI prompts at the end of a session worth sharing and the dedicated `share` workflow (`teamai skill get share`) takes over (it summarizes the session and runs `teamai contribute`). The user does not come through this flow for it. (Step A below is only a manual fallback for while recall is off.) diff --git a/skill-data/setup/references/join-member.md b/skill-data/setup/references/join-member.md index 564b2a86..0f5f5489 100644 --- a/skill-data/setup/references/join-member.md +++ b/skill-data/setup/references/join-member.md @@ -143,7 +143,8 @@ Summarize the outcome **in the user's own language** (global rule 1). Cover: summarize and contribute it. They do **not** invoke `/teamai` for this. (This prompt only appears when recall is on — it is off by default; the admin turns it on in `teamai.yaml` (`sharing.recall.enabled`), a member with `teamai recall enable` — and the admin has not switched the - reminder off in `teamai.yaml`.) + reminder off in `teamai.yaml`. It also stays silent while their teamai config + cannot be loaded; `teamai skill get share` then names the file and the error.) 3. **They can also contribute a skill — just ask in plain language.** A member does not need to be an admin to publish a skill. They tell TeamAI something like *"share this xxx skill with my team"*, in their own language, and you diff --git a/skill-data/setup/references/manage-admin.md b/skill-data/setup/references/manage-admin.md index d168ad08..c5c9f343 100644 --- a/skill-data/setup/references/manage-admin.md +++ b/skill-data/setup/references/manage-admin.md @@ -133,14 +133,16 @@ the team (`sharing.recall.enabled: true` in `teamai.yaml`, then `teamai push`; i off by default, and `teamai recall enable` turns it on for one machine only): at the end of a session worth sharing, TeamAI prompts the member and the dedicated `share` workflow (`teamai skill get share`) summarizes the session and runs -`teamai contribute`. Nobody has to invoke it by hand. +`teamai contribute`. Nobody has to invoke it by hand. (A member whose teamai config +cannot be loaded gets no prompt; `teamai skill get share` names the file and the error.) (Publishing a **reusable skill** someone authored is a different task — any member can do it, see `"$(teamai skill path core)/references/contribute-member.md"`.) ### Turn the sharing prompt on or off (admin) The auto-share prompt is **on by default once recall is on**, and only shows in directories set up -with teamai. To disable it team-wide, set this in +with teamai (never on a read-only HTTP source, or while a member's teamai config cannot be loaded). +To disable it team-wide, set this in `teamai.yaml` and `teamai push`: ```yaml diff --git a/skill-data/setup/references/setup-admin.md b/skill-data/setup/references/setup-admin.md index acabd7b9..17698f6f 100644 --- a/skill-data/setup/references/setup-admin.md +++ b/skill-data/setup/references/setup-admin.md @@ -275,7 +275,8 @@ day-to-day work — they can keep letting the AI run things for them: its own at the end of a session worth sharing, and the `share` workflow (`teamai skill get share`) takes over. (Only once recall is on — off by default; turn it on team-wide with `sharing.recall.enabled: true` in `teamai.yaml`, then - `teamai push`.) + `teamai push`. Never on a read-only HTTP source, or while a member's teamai + config cannot be loaded.) Mention the underlying commands (`teamai push`, `teamai roles`, …) only as a note for users who *do* want them — the primary path is re-invoking `/teamai`. diff --git a/src/__tests__/config-not-initialized.test.ts b/src/__tests__/config-not-initialized.test.ts index 6de26fb9..e23eb4c1 100644 --- a/src/__tests__/config-not-initialized.test.ts +++ b/src/__tests__/config-not-initialized.test.ts @@ -11,6 +11,7 @@ vi.mock('../utils/logger.js', () => ({ import { NotInitializedError, detectProjectConfig, findUnreadableProjectConfig, requireInit } from '../config.js'; import { projectDataHome } from '../utils/partition.js'; +import { log } from '../utils/logger.js'; /** * `loadLocalConfig` returns null both for a missing file and for one it could @@ -46,6 +47,17 @@ describe('requireInit: missing config versus unreadable config', () => { expect(String(error)).toContain(configPath); }); + it('says an empty config is empty, since no loader logged anything for it', async () => { + const configPath = path.join(home, '.teamai', 'config.yaml'); + fs.mkdirSync(path.dirname(configPath), { recursive: true }); + fs.writeFileSync(configPath, ''); + + const error = await requireInit().catch((e: unknown) => e); + expect(error).not.toBeInstanceOf(NotInitializedError); + expect(String(error)).toContain(`${configPath} could not be read: it is empty`); + expect(String(error)).not.toContain('above'); + }); + it('is not NotInitializedError when the config parses but fails validation', async () => { const configPath = path.join(home, '.teamai', 'config.yaml'); fs.mkdirSync(path.dirname(configPath), { recursive: true }); @@ -53,6 +65,45 @@ describe('requireInit: missing config versus unreadable config', () => { await expect(requireInit()).rejects.not.toBeInstanceOf(NotInitializedError); }); + + it('says a team config that exists but fails validation is invalid, not missing', async () => { + // "not found. Check your repo path" sends the member after a path that is right. + const teamRepo = path.join(home, '.teamai', 'team-repo'); + fs.mkdirSync(teamRepo, { recursive: true }); + fs.writeFileSync(path.join(teamRepo, 'teamai.yaml'), 'team: 42\n'); + fs.writeFileSync( + path.join(home, '.teamai', 'config.yaml'), + `repo:\n localPath: ${teamRepo}\n remote: https://example.test/acme/team.git\nusername: tester\nscope: user\n`, + ); + + const error = String(await requireInit().catch((e: unknown) => e)); + expect(error).toContain(`${path.join(teamRepo, 'teamai.yaml')} could not be read: it is not a valid team config`); + expect(error).not.toContain('not found'); + }); + + it('still says a missing team config is not found', async () => { + const teamRepo = path.join(home, '.teamai', 'team-repo'); + fs.mkdirSync(teamRepo, { recursive: true }); + fs.writeFileSync( + path.join(home, '.teamai', 'config.yaml'), + `repo:\n localPath: ${teamRepo}\n remote: https://example.test/acme/team.git\nusername: tester\nscope: user\n`, + ); + + await expect(requireInit()).rejects.toThrow('Team config (teamai.yaml) not found'); + }); + + it('logs the failing field of a config that fails validation, which the refusal points at', async () => { + // The refusal says "the error is printed above"; a Zod JSON dump there + // would bury the field under a line of `[`. + const configPath = path.join(home, '.teamai', 'config.yaml'); + fs.mkdirSync(path.dirname(configPath), { recursive: true }); + fs.writeFileSync(configPath, 'username: 42\n'); + vi.mocked(log.error).mockClear(); + + await expect(requireInit()).rejects.toThrow('the error is printed above'); + expect(vi.mocked(log.error)).toHaveBeenCalledWith(expect.stringMatching(/username: Expected string, received number/)); + expect(vi.mocked(log.error).mock.calls.flat().join('')).not.toContain('\n'); + }); }); describe('findUnreadableProjectConfig', () => { @@ -86,6 +137,63 @@ describe('findUnreadableProjectConfig', () => { expect(await findUnreadableProjectConfig(dir)).toContain(configPath); }); + it('names the failing field of a project config that parses but fails validation, on one line', async () => { + // A Zod message is a JSON dump whose first line is `[`; the refusal keeps + // only the first line, so the field and the reason must lead. + const configPath = path.join(dir, '.teamai', 'config.yaml'); + fs.mkdirSync(path.dirname(configPath), { recursive: true }); + fs.writeFileSync(configPath, 'scope: project\nrepo: 42\n'); + + const problem = await findUnreadableProjectConfig(dir); + expect(problem).toContain(configPath); + expect(problem).toMatch(/repo: Expected object, received number/); + expect(problem).not.toContain('\n'); + }); + + it('names a project config that is not scope: project, which detection alone skips', async () => { + // `scope` omitted defaults to user: detection would read past the file to + // the user config, another team's. + const configPath = path.join(dir, '.teamai', 'config.yaml'); + fs.mkdirSync(path.dirname(configPath), { recursive: true }); + fs.writeFileSync(configPath, 'repo:\n localPath: /x\n remote: https://example.test/a.git\nusername: t\n'); + + const problem = await findUnreadableProjectConfig(dir); + expect(problem).toContain(configPath); + expect(problem).toContain('scope: project'); + }); + + it('does not name the user config when the directory is HOME itself', async () => { + // Run from HOME, `/.teamai/config.yaml` is the user config, not a project's. + vi.stubEnv('HOME', dir); + vi.stubEnv('USERPROFILE', dir); + const configPath = path.join(dir, '.teamai', 'config.yaml'); + fs.mkdirSync(path.dirname(configPath), { recursive: true }); + fs.writeFileSync(configPath, 'repo:\n localPath: /x\n remote: https://example.test/a.git\nusername: t\nscope: user\n'); + + expect(await findUnreadableProjectConfig(dir)).toBeNull(); + vi.unstubAllEnvs(); + }); + + it('names a project config that is a symlink to the user config', async () => { + // The HOME exception is about where the project is, not where the file points. + const home = fs.mkdtempSync(path.join(os.tmpdir(), 'teamai-symlink-home-')); + vi.stubEnv('HOME', home); + vi.stubEnv('USERPROFILE', home); + const userConfig = path.join(home, '.teamai', 'config.yaml'); + fs.mkdirSync(path.dirname(userConfig), { recursive: true }); + fs.writeFileSync(userConfig, 'repo:\n localPath: /x\n remote: https://example.test/a.git\nusername: t\nscope: user\n'); + const configPath = path.join(dir, '.teamai', 'config.yaml'); + fs.mkdirSync(path.dirname(configPath), { recursive: true }); + fs.symlinkSync(userConfig, configPath); + + try { + expect(await findUnreadableProjectConfig(dir)).toContain(configPath); + } finally { + vi.unstubAllEnvs(); + fs.rmSync(home, { recursive: true, force: true }); + } + }); + it('names a broken partition config even when the legacy .teamai/ config behind it loads', async () => { // The partition is authoritative; detection skips it when broken and lands // on the legacy config, which may belong to another team. diff --git a/src/__tests__/contribute-check-e2e.test.ts b/src/__tests__/contribute-check-e2e.test.ts index e0078416..3eb51c89 100644 --- a/src/__tests__/contribute-check-e2e.test.ts +++ b/src/__tests__/contribute-check-e2e.test.ts @@ -18,11 +18,12 @@ const SESSION_ID = 'e2e-test-session-001'; const RAW_GITHUB_TOKEN = 'ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789'; const FIRST_TASK = `Fix auth retry for ${RAW_GITHUB_TOKEN}\nthen add regression coverage`; -/** A temp HOME with a user-scope install: the nudge only runs where teamai is set up (#748). */ +/** A temp HOME with a user-scope install and recall on: the nudge only runs where `share` is served (#748). */ function makeTmpHome(): string { const home = fs.mkdtempSync(path.join(os.tmpdir(), 'teamai-contribute-e2e-')); const teamRepo = path.join(home, '.teamai', 'team-repo'); fs.mkdirSync(teamRepo, { recursive: true }); + fs.writeFileSync(path.join(teamRepo, 'teamai.yaml'), 'team: acme\nrepo: https://example.test/acme/team.git\nsharing:\n recall:\n enabled: true\n'); fs.writeFileSync( path.join(home, '.teamai', 'config.yaml'), `repo:\n localPath: ${teamRepo}\n remote: https://example.test/acme/team.git\nusername: tester\nscope: user\n`, @@ -31,11 +32,11 @@ function makeTmpHome(): string { } /** Build a hook STDIN JSON payload with a session_id. */ -function makeStdinPayload(sessionId: string): string { +function makeStdinPayload(sessionId: string, cwd = '/tmp/fake-project'): string { return JSON.stringify({ session_id: sessionId, hook_event_name: 'Stop', - cwd: '/tmp/fake-project', + cwd, }); } @@ -66,12 +67,15 @@ function runContributeCheck( homeDir: string, stdinPayload: string, tool = 'claude', + processCwd = homeDir, ): Promise<{ stdout: string; stderr: string; code: number }> { return new Promise((resolve) => { const child = execFile( 'node', [CLI_PATH, 'contribute-check', '--stdin', '--tool', tool], { + // The directory the hook process starts in, which the gate must not read. + cwd: processCwd, env: { ...process.env, HOME: homeDir, TEAMAI_LOG_LEVEL: 'silent' }, timeout: 10000, }, @@ -212,7 +216,8 @@ describe('contribute-check E2E', () => { expect(parsed.hookSpecificOutput.additionalContext).not.toContain(RAW_GITHUB_TOKEN); expect(parsed.hookSpecificOutput.additionalContext).not.toContain('50 tool calls'); expect(parsed.hookSpecificOutput.additionalContext).not.toContain('7 different tools'); - expect(parsed.hookSpecificOutput.additionalContext).toContain('/teamai'); + expect(parsed.hookSpecificOutput.additionalContext).toContain('/teamai share what this session taught me'); + expect(parsed.hookSpecificOutput.additionalContext).toContain('teamai skill get share'); expect(parsed.stopReason).toBeUndefined(); // The real CLI persists hinted=true, so a repeated Stop hook is silent. @@ -221,6 +226,45 @@ describe('contribute-check E2E', () => { expect(repeated.stdout).toBe(''); }); + it('withholds the reminder where `teamai skill get share` refuses: a config that cannot be loaded', async () => { + // Hooks written before the dispatcher still call this command directly; it + // must ask the same gate, or it nudges towards a command that says no. + writeEventsFile(tmpHome, buildRichSessionEvents(SESSION_ID)); + fs.writeFileSync(path.join(tmpHome, '.teamai', 'config.yaml'), ''); + + const result = await runContributeCheck(tmpHome, makeStdinPayload(SESSION_ID)); + + expect(result.stdout).toBe(''); + }); + + it('asks the gate about the payload cwd, not the directory the hook process started in', async () => { + // The process starts in HOME, where the user config (recall on) would allow + // the nudge; the session ran in a project whose config does not parse, where + // `teamai skill get share` refuses. + const project = path.join(tmpHome, 'project'); + fs.mkdirSync(path.join(project, '.teamai'), { recursive: true }); + fs.writeFileSync(path.join(project, '.teamai', 'config.yaml'), 'repo: [unclosed\n'); + writeEventsFile(tmpHome, buildRichSessionEvents(SESSION_ID)); + + const result = await runContributeCheck(tmpHome, makeStdinPayload(SESSION_ID, project)); + expect(result.code).toBe(0); + expect(result.stdout).toBe(''); + }); + + it('never asks the gate about the launcher directory, even when the payload cwd no longer exists', async () => { + // The hook process starts in a project whose config does not parse; the + // session ran in a worktree since deleted, which holds no project config, + // so the user config (recall on) decides. + const launcher = path.join(tmpHome, 'launcher'); + fs.mkdirSync(path.join(launcher, '.teamai'), { recursive: true }); + fs.writeFileSync(path.join(launcher, '.teamai', 'config.yaml'), 'repo: [unclosed\n'); + writeEventsFile(tmpHome, buildRichSessionEvents(SESSION_ID)); + + const result = await runContributeCheck(tmpHome, makeStdinPayload(SESSION_ID, path.join(tmpHome, 'deleted-worktree')), 'claude', launcher); + expect(result.code).toBe(0); + expect(result.stdout).not.toBe(''); + }); + it('produces no output for a trivial session below threshold', async () => { writeEventsFile(tmpHome, buildTrivialSessionEvents(SESSION_ID)); diff --git a/src/__tests__/e2e/skill-serving-cli.test.ts b/src/__tests__/e2e/skill-serving-cli.test.ts index 2b6688e6..c9dd43e8 100644 --- a/src/__tests__/e2e/skill-serving-cli.test.ts +++ b/src/__tests__/e2e/skill-serving-cli.test.ts @@ -130,7 +130,8 @@ describe('teamai skill get / path CLI (e2e)', () => { encoding: 'utf8', }); expect(result.status, args.join(' ')).not.toBe(0); - expect(result.stderr, args.join(' ')).toContain('could not be read'); + // It names the file and where it breaks, which is what the member fixes. + expect(result.stderr, args.join(' ')).toMatch(/\.teamai[\\/]config\.yaml: .* at line \d+, column \d+/); expect(result.stdout + result.stderr, args.join(' ')).not.toContain('Not initialized'); expect(result.stdout + result.stderr, args.join(' ')).not.toContain('No team is set up'); } diff --git a/src/__tests__/git-kind-learnings.test.ts b/src/__tests__/git-kind-learnings.test.ts index 8f44ecdc..068aa54b 100644 --- a/src/__tests__/git-kind-learnings.test.ts +++ b/src/__tests__/git-kind-learnings.test.ts @@ -26,7 +26,9 @@ beforeEach(() => { afterEach(() => { process.env.HOME = originalHome; - fs.rmSync(tmp, { recursive: true, force: true }); + // A push into the bare origin can leave a detached `git gc --auto` writing to + // objects/pack after the test returns, so a single rmdir races it (ENOTEMPTY). + fs.rmSync(tmp, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); }); async function configureGit(dir: string): Promise { diff --git a/src/__tests__/hook-handlers.test.ts b/src/__tests__/hook-handlers.test.ts index 95c34fce..e2acb0d4 100644 --- a/src/__tests__/hook-handlers.test.ts +++ b/src/__tests__/hook-handlers.test.ts @@ -1,4 +1,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; // ── Mocks ──────────────────────────────────────────────── // Mock the underlying modules so handlers don't do real I/O @@ -82,9 +85,15 @@ const mockAutoDetectInit = vi.fn().mockResolvedValue({ teamConfig: { team: 'test', repo: '', toolPaths: {}, sharing: { recall: { enabled: true } } }, }); +const mockFindUnreadableProjectConfig = vi.fn().mockResolvedValue(null); + vi.mock('../config.js', async (importOriginal) => ({ ...(await importOriginal()), autoDetectInit: mockAutoDetectInit, + // A payload cwd that no longer exists (these tests use '/x') holds no + // project config, so the gate asks the user config: the same mocked one. + requireInit: mockAutoDetectInit, + findUnreadableProjectConfig: mockFindUnreadableProjectConfig, resolveConfigForDir: vi.fn().mockResolvedValue({ repo: { localPath: '/tmp/team-repo', remote: '' }, username: 'test', scope: 'user', additionalRoles: [], }), @@ -92,6 +101,7 @@ vi.mock('../config.js', async (importOriginal) => ({ vi.mock('../utils/logger.js', () => ({ log: { info: vi.fn(), success: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, + setStderrOnly: vi.fn().mockReturnValue(false), })); vi.mock('../local-agent.js', () => ({ @@ -414,7 +424,8 @@ describe('hook-handlers registry', () => { expect(mockContributeCheckForSession).not.toHaveBeenCalled(); }); - it('contribute-check handler stays silent when there is no config at all (#748)', async () => { + it('contribute-check handler stays silent when there is no config at all', async () => { + // A project that never set up teamai has no team to share with (#748). const { NotInitializedError } = await import('../config.js'); const registry = buildHandlerRegistry(); const handler = registry.find( @@ -429,6 +440,77 @@ describe('hook-handlers registry', () => { expect(mockContributeCheckForSession).not.toHaveBeenCalled(); }); + it('contribute-check handler stays silent when the project config is unreadable, even if the user config loads', async () => { + const registry = buildHandlerRegistry(); + const handler = registry.find( + (r) => r.event === 'stop' && r.handler.name === 'contribute-check', + )!.handler; + // Detection skips the broken project file and loads the user config (recall + // on), but `teamai skill get share` refuses here, so the nudge would lead nowhere. + mockFindUnreadableProjectConfig.mockResolvedValueOnce('/x/.teamai/config.yaml: bad indentation'); + mockContributeCheckForSession.mockClear(); + + // An existing directory: a deleted one holds no project config to be unreadable. + const result = await handler.execute({ session_id: 's5c', cwd: process.cwd() }, 'claude'); + expect(result).toBeNull(); + expect(mockContributeCheckForSession).not.toHaveBeenCalled(); + }); + + it('contribute-check handler asks the gate about the payload cwd, not the directory the process is in', async () => { + // hook-dispatch changes into the payload cwd, but that can fail; the gate + // must not then read wherever the process started. + const registry = buildHandlerRegistry(); + const handler = registry.find( + (r) => r.event === 'stop' && r.handler.name === 'contribute-check', + )!.handler; + mockFindUnreadableProjectConfig.mockImplementation(async (cwd?: string) => + cwd === undefined ? '/launcher/.teamai/config.yaml: bad indentation' : null); + mockContributeCheckForSession.mockResolvedValueOnce({ hint: '[teamai] do share' }); + try { + const result = await handler.execute({ session_id: 's5e', cwd: process.cwd() }, 'claude'); + expect(result).toContain('do share'); + } finally { + mockFindUnreadableProjectConfig.mockReset(); + mockFindUnreadableProjectConfig.mockResolvedValue(null); + } + }); + + it('contribute-check handler withholds the reminder when the payload cwd exists but cannot be checked', async () => { + // Only a cwd that is gone (ENOENT) falls back to the user config; one that + // cannot be opened, here a path through a file (ENOTDIR), is unknown. + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'teamai-hook-cwd-')); + const file = path.join(dir, 'a-file'); + fs.writeFileSync(file, ''); + const registry = buildHandlerRegistry(); + const handler = registry.find( + (r) => r.event === 'stop' && r.handler.name === 'contribute-check', + )!.handler; + mockContributeCheckForSession.mockClear(); + try { + const result = await handler.execute({ session_id: 's5f', cwd: path.join(file, 'sub') }, 'claude'); + expect(result).toBeNull(); + expect(mockContributeCheckForSession).not.toHaveBeenCalled(); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('contribute-check handler withholds the reminder, without failing the turn, when the gate itself faults', async () => { + const registry = buildHandlerRegistry(); + const handler = registry.find( + (r) => r.event === 'stop' && r.handler.name === 'contribute-check', + )!.handler; + mockAutoDetectInit.mockResolvedValueOnce({ + localConfig: { get repo(): never { throw new TypeError('a bug past the config load'); } }, + teamConfig: { team: 'test', repo: '', toolPaths: {}, sharing: { recall: { enabled: true } } }, + }); + mockContributeCheckForSession.mockClear(); + + const result = await handler.execute({ session_id: 's5d', cwd: '/x' }, 'claude'); + expect(result).toBeNull(); + expect(mockContributeCheckForSession).not.toHaveBeenCalled(); + }); + it('contribute-check handler stays silent when a config exists but cannot be loaded', async () => { const registry = buildHandlerRegistry(); const handler = registry.find( diff --git a/src/__tests__/init.test.ts b/src/__tests__/init.test.ts index fc82d453..0fa582ea 100644 --- a/src/__tests__/init.test.ts +++ b/src/__tests__/init.test.ts @@ -564,19 +564,14 @@ describe('init', () => { }); describe('deploys built-in skills after init', () => { - it('calls deployBuiltinSkills with teamConfig when loadTeamConfig returns non-null', async () => { + /** Init against a clone whose teamai.yaml loads, so the stub deploy runs. */ + async function initWithTeamConfig(): Promise { let cloneDone = false; - pathExistsFn = (p: string) => { - if (p === localPath) return cloneDone; - return false; - }; - + pathExistsFn = (p: string) => (p === localPath ? cloneDone : false); mockGfRepoClone.mockImplementation(() => { cloneDone = true; }); - - const mockedLoadTeamConfig = vi.mocked(await import('../config.js')).loadTeamConfig; - mockedLoadTeamConfig.mockResolvedValue({ + vi.mocked(await import('../config.js')).loadTeamConfig.mockResolvedValue({ team: 'my-team', repo: 'https://git.woa.com/HyperAI/teamai-test.git', provider: 'tgit', @@ -589,10 +584,12 @@ describe('init', () => { }, toolPaths: {}, } as never); - questionAnswers = ['n', '1']; - await init({ repo: 'https://git.woa.com/HyperAI/teamai-test.git', scope: 'user' }); + } + + it('calls deployBuiltinSkills with teamConfig when loadTeamConfig returns non-null', async () => { + await initWithTeamConfig(); expect(mockDeployBuiltinSkills).toHaveBeenCalled(); // No recall option: one stub deploys for everyone, and `teamai skill get @@ -602,6 +599,33 @@ describe('init', () => { expect.anything(), ); }); + + it('announces the stub as ready only when it landed', async () => { + const { log } = await import('../utils/logger.js'); + mockDeployBuiltinSkills.mockResolvedValueOnce(1); + + await initWithTeamConfig(); + + expect(log.info).toHaveBeenCalledWith(expect.stringContaining('The built-in teamai skill is ready in your IDE')); + expect(log.warn).not.toHaveBeenCalledWith(expect.stringContaining('not deployed')); + }); + + it('says the stub reached no tool without pointing at output that may not exist', async () => { + // No installed tool logs at debug only, so "see the lines above" alone + // would be false, and `teamai doctor` has no check for the stub. + const { log } = await import('../utils/logger.js'); + mockDeployBuiltinSkills.mockResolvedValueOnce(0); + + await initWithTeamConfig(); + + const warned = vi.mocked(log.warn).mock.calls.map((call) => String(call[0])).join('\n'); + expect(warned).toContain('The built-in teamai skill was not deployed to any AI tool'); + expect(warned).toContain('teamai pull'); + expect(warned).toContain('~/.teamai/debug.log'); + expect(warned).not.toContain('see the lines above'); + expect(warned).not.toContain('teamai doctor'); + expect(log.info).not.toHaveBeenCalledWith(expect.stringContaining('is ready in your IDE')); + }); }); describe('scope path display', () => { diff --git a/src/__tests__/logger.test.ts b/src/__tests__/logger.test.ts index ed57c00c..7884d038 100644 --- a/src/__tests__/logger.test.ts +++ b/src/__tests__/logger.test.ts @@ -45,6 +45,28 @@ describe('file transport', () => { expect(fs.readFileSync(logFile, 'utf-8')).toContain('[ERROR] something broke'); }); + it('persists to file only, never the console, even when silent', () => { + // A detached SessionStart pull runs silent with its output discarded; the + // failures it cannot show still leave a record, printed nowhere twice. + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + setVerbose(true); + setSilent(true); + log.persist('stub not deployed'); + expect(logSpy).not.toHaveBeenCalled(); + expect(errorSpy).not.toHaveBeenCalled(); + logSpy.mockRestore(); + errorSpy.mockRestore(); + expect(fs.readFileSync(logFile, 'utf-8')).toContain('[WARN] stub not deployed'); + }); + + it('keeps warn on the console only', () => { + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + log.warn('shown, not stored'); + logSpy.mockRestore(); + expect(fs.existsSync(logFile) ? fs.readFileSync(logFile, 'utf-8') : '').not.toContain('shown, not stored'); + }); + it('includes timestamp', () => { log.debug('ts'); expect(fs.readFileSync(logFile, 'utf-8').trim()).toMatch(/^\d{4}-\d{2}-\d{2}T/); diff --git a/src/__tests__/pull-skip-sync.test.ts b/src/__tests__/pull-skip-sync.test.ts index bbb7e235..f2ae089d 100644 --- a/src/__tests__/pull-skip-sync.test.ts +++ b/src/__tests__/pull-skip-sync.test.ts @@ -24,6 +24,7 @@ vi.mock('../utils/git.js', () => ({ vi.mock('../utils/logger.js', () => ({ log: { + persist: vi.fn(), info: vi.fn(), success: vi.fn(), warn: vi.fn(), @@ -81,6 +82,12 @@ vi.mock('../update.js', () => ({ releaseLock: vi.fn().mockResolvedValue(undefined), })); +// The real deploy by default; a test makes it fail once to see what pull reports. +vi.mock('../builtin-skills.js', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, deployBuiltinSkills: vi.fn(actual.deployBuiltinSkills) }; +}); + import { pull, compileRecallRulesBlock, cleanupInactiveNamespaceSkills } from '../pull.js'; import { loadLocalConfigForScope, loadTeamConfig, detectProjectConfig, loadStateForScope, saveStateForScope } from '../config.js'; import { getHeadRev, createGit, pullRepo } from '../utils/git.js'; @@ -300,6 +307,37 @@ describe('pull skip-sync when repo HEAD unchanged', () => { expect(vi.mocked(saveStateForScope).mock.calls[0][0].lastPullTargets).toEqual(['claude']); }); + it('warns when the built-in stub cannot be deployed on the revision fast path', async () => { + // The stub is the agent's only way into TeamAI: a failure to write it must + // reach the member, not vanish after "Already synced" has printed. + const { deployBuiltinSkills } = await import('../builtin-skills.js'); + vi.mocked(deployBuiltinSkills).mockRejectedValueOnce(new Error('EACCES: permission denied')); + vi.mocked(getHeadRev).mockResolvedValue('abc1234'); + vi.mocked(loadStateForScope).mockResolvedValue(emptyState({ lastPullRev: 'abc1234', lastPullTargets: ['claude'] })); + + await pull({}); + + expect(log.success).toHaveBeenCalledWith(expect.stringContaining('Already synced at abc1234, skipping')); + expect(log.warn).toHaveBeenCalledWith(expect.stringContaining('The built-in teamai skill was not deployed')); + expect(log.warn).toHaveBeenCalledWith(expect.stringContaining('EACCES: permission denied')); + // A detached SessionStart pull discards its output: debug.log keeps the record. + expect(log.persist).toHaveBeenCalledWith(expect.stringContaining('The built-in teamai skill was not deployed: EACCES')); + }); + + it('warns when the built-in stub cannot be deployed on a full sync', async () => { + const { deployBuiltinSkills } = await import('../builtin-skills.js'); + vi.mocked(deployBuiltinSkills).mockRejectedValueOnce(new Error('EACCES: permission denied')); + vi.mocked(getHeadRev).mockResolvedValue('def5678'); + vi.mocked(loadStateForScope).mockResolvedValue(emptyState({ lastPullRev: 'abc1234' })); + + await pull({}); + + expect(log.warn).toHaveBeenCalledWith(expect.stringContaining('The built-in teamai skill was not deployed')); + expect(log.warn).toHaveBeenCalledWith(expect.stringContaining('EACCES: permission denied')); + // A detached SessionStart pull discards its output: debug.log keeps the record. + expect(log.persist).toHaveBeenCalledWith(expect.stringContaining('The built-in teamai skill was not deployed: EACCES')); + }); + it('should do full sync when HEAD rev differs from lastPullRev', async () => { await fse.writeFile(path.join(repoPath, 'rules', 'my-rule.md'), '# rule'); diff --git a/src/__tests__/skill-commands-exist.test.ts b/src/__tests__/skill-commands-exist.test.ts index f4180f57..62a2d2da 100644 --- a/src/__tests__/skill-commands-exist.test.ts +++ b/src/__tests__/skill-commands-exist.test.ts @@ -92,6 +92,18 @@ function validate(program: Command, invocations: Invocation[]): string[] { continue; } + // A group with subcommands and no argument of its own can only be followed + // by one of them (or a flag), so any other word is a subcommand that does + // not exist. Placeholders such as `` stand for one and are skipped. + const next = rest[0]; + if ( + next !== undefined && command.commands.length > 0 && command.registeredArguments.length === 0 + && !next.startsWith('-') && !/[<>[\]{}"'…]/.test(next) + ) { + problems.push(`${where} → unknown subcommand "${next}" for \`teamai ${command.name()}\``); + continue; + } + const flags = knownFlags(command, program); for (const token of rest) { if (!token.startsWith('-') || token === '-') continue; @@ -125,10 +137,16 @@ describe('commands named by the served skill content', () => { const problems = validate(program, [ { file: 'synthetic.md', line: 1, text: 'teamai extract graph' }, { file: 'synthetic.md', line: 2, text: 'teamai codebase --no-such-flag' }, + // A misspelled or renamed subcommand inside a group: the group matches, so + // without its own check the typo is read as an argument. + { file: 'synthetic.md', line: 3, text: 'teamai skill gett core' }, + // A group that takes an argument of its own is left alone: `enabel` is a query. + { file: 'synthetic.md', line: 4, text: 'teamai recall enabel' }, ]); - expect(problems).toHaveLength(2); + expect(problems).toHaveLength(3); expect(problems[0]).toContain('unknown command "extract"'); expect(problems[1]).toContain('unknown flag "--no-such-flag"'); + expect(problems[2]).toContain('unknown subcommand "gett" for `teamai skill`'); }); }); diff --git a/src/__tests__/skill-list-uninitialized.test.ts b/src/__tests__/skill-list-uninitialized.test.ts index d6b4384f..a68ab4e8 100644 --- a/src/__tests__/skill-list-uninitialized.test.ts +++ b/src/__tests__/skill-list-uninitialized.test.ts @@ -1,13 +1,21 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -const { autoDetectInit, logDim, NotInitializedError } = vi.hoisted(() => ({ +const { autoDetectInit, findUnreadableProjectConfig, logDim, logError, NotInitializedError } = vi.hoisted(() => ({ autoDetectInit: vi.fn(), + findUnreadableProjectConfig: vi.fn(), logDim: vi.fn(), + logError: vi.fn(), NotInitializedError: class NotInitializedError extends Error {}, })); -vi.mock('../config.js', () => ({ autoDetectInit, NotInitializedError })); +vi.mock('../config.js', () => ({ + autoDetectInit, + findUnreadableProjectConfig, + requireInit: vi.fn(), + NotInitializedError, + BROKEN_CONFIG_ADVICE: 'Fix the file, or move it aside and run `teamai init` to write a new one.', +})); vi.mock('../utils/logger.js', () => ({ - log: { info: vi.fn(), success: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn(), dim: logDim }, + log: { info: vi.fn(), success: vi.fn(), warn: vi.fn(), error: logError, debug: vi.fn(), dim: logDim }, setStderrOnly: vi.fn(() => false), })); @@ -25,7 +33,11 @@ describe('teamai skill list before init', () => { beforeEach(() => { stdout = ''; autoDetectInit.mockReset(); + // No project config under the test's cwd: detection goes on to autoDetectInit. + findUnreadableProjectConfig.mockReset(); + findUnreadableProjectConfig.mockResolvedValue(null); logDim.mockReset(); + logError.mockReset(); logSpy = vi.spyOn(console, 'log').mockImplementation((...args: unknown[]) => { stdout += args.join(' ') + '\n'; }); @@ -54,7 +66,33 @@ describe('teamai skill list before init', () => { // member to run `teamai init` would send them to re-init over a real setup. autoDetectInit.mockRejectedValue(new Error('Team config (teamai.yaml) not found. Check your repo path.')); - await expect(skillList({})).rejects.toThrow('Team config (teamai.yaml) not found'); + await skillList({}); + + expect(process.exitCode).toBe(1); + expect(logError).toHaveBeenCalledWith(expect.stringContaining('Team config (teamai.yaml) not found')); expect(logDim).not.toHaveBeenCalledWith(expect.stringContaining('Not initialized')); + // The packaged catalog needs no team, so it is still listed. + expect(stdout).toContain('teamai skill get core'); + }); + + it('loads the config once, so a broken one is reported once', async () => { + autoDetectInit.mockRejectedValue(new Error('The teamai config at /h/.teamai/config.yaml could not be read: it is empty.')); + + await skillList({}); + + expect(autoDetectInit).toHaveBeenCalledTimes(1); + }); + + it('does not list the team the user config names while the project config is unreadable', async () => { + // Detection skips the broken project file and would answer with the user + // config: another team's repo. + findUnreadableProjectConfig.mockResolvedValue('/work/proj/.teamai/config.yaml: bad indentation'); + + await skillList({}); + + expect(autoDetectInit).not.toHaveBeenCalled(); + expect(process.exitCode).toBe(1); + expect(logError).toHaveBeenCalledWith(expect.stringContaining('/work/proj/.teamai/config.yaml: bad indentation')); + expect(stdout).toContain('teamai skill get core'); }); }); diff --git a/src/__tests__/skill-recall-gate.test.ts b/src/__tests__/skill-recall-gate.test.ts index 3fa9223c..602298fc 100644 --- a/src/__tests__/skill-recall-gate.test.ts +++ b/src/__tests__/skill-recall-gate.test.ts @@ -168,11 +168,17 @@ describe('recall gate on served skills', () => { it('blocks share when a config exists but cannot be loaded, since recall and the source are then unknown', async () => { autoDetectInit.mockRejectedValue(new Error('Team config (teamai.yaml) not found. Check your repo path.')); - expect(await resolveServableSkill('share')).toEqual({ kind: 'blocked', name: 'share', reason: 'config' }); + expect(await resolveServableSkill('share')).toEqual({ + kind: 'blocked', name: 'share', reason: 'config', + detail: 'Team config (teamai.yaml) not found. Check your repo path.', + }); await skillGet(['share']); expect(process.exitCode).toBe(1); expect(stdout).toBe(''); expect(stderr).toContain('config on this machine could not be loaded'); + // The refusal names what failed: `teamai doctor` cannot see a broken config. + expect(stderr).toContain('Team config (teamai.yaml) not found. Check your repo path.'); + expect(stderr).not.toContain('teamai doctor'); expect((await skillCatalog()).find((entry) => entry.name === 'share')).toMatchObject({ blockedBy: 'config', path: null }); // Only share depends on the config; the rest is still served. expect((await resolveServableSkill('core')).kind).toBe('found'); @@ -184,7 +190,42 @@ describe('recall gate on served skills', () => { findUnreadableProjectConfig.mockResolvedValue('/work/proj/.teamai/config.yaml: bad indentation'); withRecall(true); - expect(await resolveServableSkill('share')).toEqual({ kind: 'blocked', name: 'share', reason: 'config' }); + expect(await resolveServableSkill('share')).toEqual({ + kind: 'blocked', name: 'share', reason: 'config', + detail: '/work/proj/.teamai/config.yaml: bad indentation. ' + + 'Fix the file, or move it aside and run `teamai init` to write a new one.', + }); expect(autoDetectInit).not.toHaveBeenCalled(); + + await skillGet(['share']); + expect(process.exitCode).toBe(1); + expect(stderr).toContain('/work/proj/.teamai/config.yaml: bad indentation'); + expect(stderr).toContain('move it aside'); + }); + + it('keeps only the first line of a multi-line parse error, which names the file and the position', async () => { + // YAML errors end in a code frame and a newline; appended as-is, the + // advice would start a line of its own with a stray ". ". + findUnreadableProjectConfig.mockResolvedValue( + '/work/proj/.teamai/config.yaml: Unexpected flow-seq-end at line 1, column 7:\n\nrepo: [unclosed\n ^\n', + ); + + expect(await resolveServableSkill('share')).toMatchObject({ + reason: 'config', + detail: '/work/proj/.teamai/config.yaml: Unexpected flow-seq-end at line 1, column 7. ' + + 'Fix the file, or move it aside and run `teamai init` to write a new one.', + }); + }); + + it('lets a fault in the gate itself propagate instead of reporting it as a broken config', async () => { + // Only loading the config means "cannot be loaded"; anything else would + // print "the teamai config could not be loaded" over an unrelated bug. + const fault = new TypeError('a bug past the config load'); + autoDetectInit.mockResolvedValue({ + localConfig: { get repo(): never { throw fault; } }, + teamConfig: { sharing: { recall: { enabled: true } } }, + }); + + await expect(resolveServableSkill('share')).rejects.toBe(fault); }); }); diff --git a/src/__tests__/skill-show.test.ts b/src/__tests__/skill-show.test.ts index 616c6cfd..0263114a 100644 --- a/src/__tests__/skill-show.test.ts +++ b/src/__tests__/skill-show.test.ts @@ -86,10 +86,19 @@ function captureLogs() { }; } -async function runSkillShow(name: string, fx: Fixture): Promise { +async function runSkillShow( + name: string, + fx: Fixture, + config: { unreadableProjectConfig?: string; loadError?: Error; loads?: { count: number } } = {}, +): Promise { vi.doMock('../config.js', async (importOriginal) => ({ ...(await importOriginal()), - autoDetectInit: async () => ({ localConfig: fx.localConfig, teamConfig: fx.teamConfig }), + autoDetectInit: async () => { + if (config.loads) config.loads.count += 1; + if (config.loadError) throw config.loadError; + return { localConfig: fx.localConfig, teamConfig: fx.teamConfig }; + }, + findUnreadableProjectConfig: async () => config.unreadableProjectConfig ?? null, })); const { skillShow } = await import('../skill-cmd.js'); const cap = captureLogs(); @@ -198,6 +207,60 @@ describe('skillShow locator', () => { process.exitCode = 0; }); + it('refuses share under a broken project config before searching the user config it falls back to', async () => { + // Detection skips the broken project file and answers with the user config: + // another team's repo and agents, where a `share` directory is not the one + // this project would mean. + fx.localConfig.recallEnabled = true; + await makeSkill(path.join(fx.repoPath, 'skills'), 'share', 'the other team share skill'); + await makeSkill(path.join(fx.homeDir, '.claude', 'skills'), 'share', 'a user-scope share skill'); + + const stderr: string[] = []; + const errorSpy = vi.spyOn(console, 'error').mockImplementation((...args: unknown[]) => { + stderr.push(args.join(' ')); + }); + let text: string; + try { + text = (await runSkillShow('share', fx, { unreadableProjectConfig: '/work/proj/.teamai/config.yaml: bad indentation' })).join('\n'); + } finally { + errorSpy.mockRestore(); + } + expect(process.exitCode).toBe(1); + expect(text).not.toContain('skill: share'); + expect(text).not.toContain(fx.repoPath); + expect(stderr.join('\n')).toContain('/work/proj/.teamai/config.yaml: bad indentation'); + process.exitCode = 0; + }); + + it('does not search the team the user config names while the project config is unreadable', async () => { + await makeSkill(path.join(fx.repoPath, 'skills'), 'other-team-skill', 'belongs to the user-scope team'); + + const text = (await runSkillShow('other-team-skill', fx, { unreadableProjectConfig: '/work/proj/.teamai/config.yaml: bad indentation' })).join('\n'); + const { log } = await import('../utils/logger.js'); + expect(process.exitCode).toBe(1); + expect(text).not.toContain(fx.repoPath); + expect(vi.mocked(log.dim)).toHaveBeenCalledWith(expect.stringContaining('/work/proj/.teamai/config.yaml: bad indentation')); + process.exitCode = 0; + }); + + it('shows a packaged skill when the config cannot be loaded, and says what failed instead of throwing', async () => { + const loadError = new Error('The teamai config at /h/.teamai/config.yaml could not be read: it is empty.'); + + const text = (await runSkillShow('core', fx, { loadError })).join('\n'); + const { log } = await import('../utils/logger.js'); + expect(text).toContain('skill: core'); + expect(vi.mocked(log.error)).toHaveBeenCalledWith(expect.stringContaining('could not be read: it is empty')); + expect(process.exitCode).toBe(1); + process.exitCode = 0; + }); + + it('loads the config once for share, so a broken one is reported once', async () => { + fx.localConfig.recallEnabled = true; + const loads = { count: 0 }; + await runSkillShow('share', fx, { loads }); + expect(loads.count).toBe(1); + }); + it('shows share once recall is enabled', async () => { fx.localConfig.recallEnabled = true; const lines = await runSkillShow('share', fx); diff --git a/src/builtin-skills.ts b/src/builtin-skills.ts index 99dcdbec..c942d18b 100644 --- a/src/builtin-skills.ts +++ b/src/builtin-skills.ts @@ -445,7 +445,10 @@ export async function pruneLegacyBuiltinSkills( log.warn(`Kept "${legacyName}" (${tool}): ${dir} holds files TeamAI did not put there. The packaged files were removed${saved}; delete the rest yourself once you have saved what you need.`); } } catch (e) { - log.debug(`Could not remove legacy built-in skill ${legacyName} from ${tool}: ${(e as Error).message}`); + const message = `Could not finish removing "${legacyName}" (${tool}) from ${dir}: ${e instanceof Error ? e.message : String(e)}. Whatever is left there stays until the next pull, which tries again.`; + log.warn(message); + // A detached SessionStart pull discards its output; debug.log keeps the record. + log.persist(message); } } } @@ -517,7 +520,8 @@ export async function deployBuiltinSkills(teamConfig: TeamaiConfig, localConfig? let entries: string[]; try { entries = await fs.promises.readdir(builtinDir); - } catch { + } catch (e) { + log.debug(`Could not list the built-in skills in ${builtinDir}, skipping deployment: ${e instanceof Error ? e.message : String(e)}`); return 0; } @@ -530,7 +534,10 @@ export async function deployBuiltinSkills(teamConfig: TeamaiConfig, localConfig? } } - if (skillNames.length === 0) return 0; + if (skillNames.length === 0) { + log.debug(`No built-in skill in ${builtinDir} has a SKILL.md, skipping deployment`); + return 0; + } let deployed = 0; diff --git a/src/config.ts b/src/config.ts index 0ec0bb62..eedd6a25 100644 --- a/src/config.ts +++ b/src/config.ts @@ -1,4 +1,6 @@ import YAML from 'yaml'; +import { ZodError } from 'zod'; +import fs from 'node:fs'; import path from 'node:path'; import { TeamaiConfigSchema, @@ -17,6 +19,7 @@ import { } from './types.js'; import { readFileSafe, readJson, writeFile, writeJson, expandHome, pathExists } from './utils/fs.js'; import { resolveAnchors } from './utils/git.js'; +import { getUserHome } from './utils/home.js'; import { resolvePartitionDir, writeAnchorFile } from './utils/partition.js'; import { log } from './utils/logger.js'; import { loadRolesManifest, RolesManifestNotFoundError } from './roles.js'; @@ -71,7 +74,7 @@ export async function loadTeamConfig(repoPath: string): Promise { const parsed = LocalConfigSchema.parse(raw); return await migrateLegacyRoleConfig(parsed, configPath); } catch (e) { - log.error(`Invalid local config: ${(e as Error).message}`); + log.error(`Invalid local config: ${describeConfigError(e)}`); return null; } } @@ -138,29 +141,57 @@ export class NotInitializedError extends Error { readonly name = 'NotInitializedError'; } +/** The local config and the team config it points at, as the init checks return them. */ +export type TeamaiInit = { localConfig: LocalConfig; teamConfig: TeamaiConfig }; + +/** What to do about a config file that exists but cannot be used. */ +export const BROKEN_CONFIG_ADVICE = 'Fix the file, or move it aside and run `teamai init` to write a new one.'; + /** * Require that teamai is initialized (local config exists) */ -export async function requireInit(): Promise<{ localConfig: LocalConfig; teamConfig: TeamaiConfig }> { +export async function requireInit(): Promise { const localConfig = await loadLocalConfig(); if (!localConfig) return throwMissingOrInvalid(expandHome(getUserConfigPath())); const teamConfig = await loadTeamConfig(localConfig.repo.localPath); - if (!teamConfig) { - throw new Error('Team config (teamai.yaml) not found. Check your repo path.'); - } + if (!teamConfig) return throwTeamConfigMissingOrInvalid(localConfig.repo.localPath); return { localConfig, teamConfig }; } /** * The loaders return null both when the config file is absent and when it could - * not be used (they log the reason). Only the first is "not initialized"; - * telling a member with a broken config to re-init sends them over a real setup. + * not be used. Only the first is "not initialized"; telling a member with a + * broken config to re-init sends them over a real setup. The loaders log a + * parse or validation error, but not a file that is empty or cannot be opened, + * so those two are named here. + */ +async function throwMissingOrInvalid(configPath: string): Promise { + if (!(await pathExists(configPath))) { + throw new NotInitializedError('teamai is not initialized. Run `teamai init` first.'); + } + const content = await readFileSafe(configPath); + const why = content === null ? 'the file could not be opened' + : content.trim() === '' ? 'it is empty' + : 'it is not a valid teamai config (the error is printed above)'; + throw new Error(`The teamai config at ${configPath} could not be read: ${why}. ${BROKEN_CONFIG_ADVICE}`); +} + +/** + * `loadTeamConfig` returns null both when teamai.yaml is absent and when it + * could not be used (it logs a parse or validation error). Only the first is + * "not found": "check your repo path" sends the member after a path that is + * right. */ -async function throwMissingOrInvalid(configPath: string, notInitializedMessage = 'teamai is not initialized. Run `teamai init` first.'): Promise { - if (await pathExists(configPath)) { - throw new Error(`The teamai config at ${configPath} could not be read (the reason is logged above). Fix the file, or move it aside and run \`teamai init\` to write a new one.`); +async function throwTeamConfigMissingOrInvalid(repoPath: string): Promise { + const teamConfigPath = path.join(repoPath, 'teamai.yaml'); + if (!(await pathExists(teamConfigPath))) { + throw new Error('Team config (teamai.yaml) not found. Check your repo path.'); } - throw new NotInitializedError(notInitializedMessage); + const content = await readFileSafe(teamConfigPath); + const why = content === null ? 'the file could not be opened' + : content.trim() === '' ? 'it is empty' + : 'it is not a valid team config (the error is printed above)'; + throw new Error(`The team config at ${teamConfigPath} could not be read: ${why}. Fix it in the team repo, or ask a team admin to.`); } // ─── Scope-aware config loading ───────────────────────── @@ -192,7 +223,7 @@ export async function loadLocalConfigForScope( const parsed = LocalConfigSchema.parse(raw); return await migrateLegacyRoleConfig(parsed, configPath); } catch (e) { - log.error(`Invalid ${scope} config at ${configPath}: ${(e as Error).message}`); + log.error(`Invalid ${scope} config at ${configPath}: ${describeConfigError(e)}`); return null; } } @@ -431,7 +462,15 @@ export async function readConfigFrom( try { const raw = YAML.parse(content); const config = LocalConfigSchema.parse(raw); - if (config.scope !== 'project') return null; + if (config.scope !== 'project') { + // Run from HOME, `/.teamai/config.yaml` is the user config itself. + // Anywhere else a config here that is not scope: project cannot say which + // project it serves, and detection would read past it to the user config. + if (!isUserTeamaiDir(dataHomeDir, projectRoot)) { + onUnreadable?.(configPath, `it is scope: ${config.scope}, but a config inside a project must be scope: project`); + } + return null; + } // Anchor projectRoot to the workspace root (resource landing) and dataHome to // the directory this config lives in (machine-data location). A persisted // projectRoot can be wrong (e.g. a `.teamai/` copied from the main checkout @@ -456,11 +495,40 @@ export async function readConfigFrom( } return resolved; } catch (e) { - onUnreadable?.(configPath, (e as Error).message); + onUnreadable?.(configPath, describeConfigError(e)); return null; } } +/** + * Whether `dataHomeDir` is `/.teamai`, decided by where the project is: + * its root is HOME. Real paths, since a tmp HOME and the cwd can differ by a + * symlink; but never the file's target, or a project config symlinked to the + * user config would pass for it. + */ +function isUserTeamaiDir(dataHomeDir: string, projectRoot: string): boolean { + const real = (p: string): string => { + try { + return fs.realpathSync(p); + } catch { + return path.resolve(p); + } + }; + return path.resolve(dataHomeDir) === path.join(path.resolve(projectRoot), '.teamai') + && real(projectRoot) === real(getUserHome()); +} + +/** + * One line naming what is wrong. A Zod message is a JSON dump of its issues, + * whose first line is `[`; each issue's field and reason is what a member fixes. + */ +function describeConfigError(e: unknown): string { + if (e instanceof ZodError) { + return e.issues.map((issue) => `${issue.path.join('.') || '(root)'}: ${issue.message}`).join('; '); + } + return e instanceof Error ? e.message : String(e); +} + /** * The project-scope config under `cwd` that exists but cannot be read, parsed * or validated, with the reason, or null. Detection skips such a file and falls @@ -492,9 +560,7 @@ export async function requireInitForScope( return throwMissingOrInvalid(expandHome(getConfigPath(scope, projectRoot))); } const teamConfig = await loadTeamConfig(localConfig.repo.localPath); - if (!teamConfig) { - throw new Error('Team config (teamai.yaml) not found. Check your repo path.'); - } + if (!teamConfig) return throwTeamConfigMissingOrInvalid(localConfig.repo.localPath); return { localConfig, teamConfig }; } @@ -503,13 +569,11 @@ export async function requireInitForScope( * If cwd has a project-scope config, uses that; otherwise falls back to user scope. * This is the recommended entry point for commands that support both scopes. */ -export async function autoDetectInit(): Promise<{ localConfig: LocalConfig; teamConfig: TeamaiConfig }> { - const projectConfig = await detectProjectConfig(); +export async function autoDetectInit(cwd?: string): Promise { + const projectConfig = await detectProjectConfig(cwd); if (projectConfig) { const teamConfig = await loadTeamConfig(projectConfig.repo.localPath); - if (!teamConfig) { - throw new Error('Team config (teamai.yaml) not found. Check your repo path.'); - } + if (!teamConfig) return throwTeamConfigMissingOrInvalid(projectConfig.repo.localPath); return { localConfig: projectConfig, teamConfig }; } return requireInit(); diff --git a/src/contribute-check.ts b/src/contribute-check.ts index 5531b9d8..a5f1d548 100644 --- a/src/contribute-check.ts +++ b/src/contribute-check.ts @@ -717,6 +717,12 @@ export async function contributeCheck(toolArg?: string): Promise { return; } + // The same gate as the dispatcher's handler: hooks written before it still + // call this command, and must not nudge towards a `share` that refuses. It + // is asked about the session's cwd, never the one this process started in. + const { contributeHintAllowed } = await import('./skill-content.js'); + if (!(await contributeHintAllowed(stdinData.cwd))) return; + const { stopStdoutUnsupported } = await import('./utils/tool-names.js'); const tool = toolArg?.toLowerCase() ?? 'claude'; const { hint } = await contributeCheckForSession( diff --git a/src/hook-handlers.ts b/src/hook-handlers.ts index 2c7e40d1..cf1266b3 100644 --- a/src/hook-handlers.ts +++ b/src/hook-handlers.ts @@ -136,8 +136,7 @@ const updateHandler: HookHandler = { /** * Team course-correction keywords for the current project. The dispatcher has * already chdir'd to the hook payload's cwd (hook-dispatch-cli), so - * autoDetectInit() resolves the right project, as it does for - * contributeHintAllowed. Only prompt hooks pay for the config read; an + * autoDetectInit() resolves the right project. Only prompt hooks pay for the config read; an * unreadable config means "built-in keywords only". */ async function teamCorrectionKeywords(stdin: Record): Promise { @@ -243,32 +242,6 @@ const trackSlashHandler: HookHandler = { }, }; -/** - * Whether the share-learnings hint may be emitted at all. Resolved lazily per - * hook run so a team can switch it off via teamai.yaml (or a member via local - * config) without re-injecting hooks. Withheld when there is no config at all: - * the hook fires in every project on the machine, and a directory without teamai - * has no team to share with (#748). A config that exists but cannot be loaded - * withholds it too: `share` refuses there, so the nudge would lead nowhere. - * - * Recall and a writable source gate it too: the hint routes to the `share` - * workflow, and `teamai skill get share` refuses while recall is off or the - * team source is read-only HTTP, so a nudge towards it would send the agent to - * a command that says no. The dispatcher already drops this `gitOnly` handler - * for HTTP teams; the check here keeps the gate the same wherever it is called. - */ -async function contributeHintAllowed(): Promise { - const { isContributeHintEnabled, isRecallEnabled } = await import('./types.js'); - const { autoDetectInit } = await import('./config.js'); - try { - const { localConfig, teamConfig } = await autoDetectInit(); - return localConfig.repo?.kind !== 'http' - && isContributeHintEnabled(localConfig, teamConfig) - && isRecallEnabled(localConfig, teamConfig); - } catch { - return false; - } -} /** * Ask the model to declare which recalled documents it actually used. @@ -289,7 +262,10 @@ export function buildVotesNudge(recalledDocIds: readonly string[]): string { const contributeCheckHandler: HookHandler = { name: 'contribute-check', async execute(stdin, tool) { - if (!(await contributeHintAllowed())) return null; + // The payload's cwd, not the process's: hook-dispatch changes into it, but + // that can fail, and the gate must not then read the launcher's directory. + const { contributeHintAllowed } = await import('./skill-content.js'); + if (!(await contributeHintAllowed(resolveHookCwd(stdin)))) return null; const { contributeCheckForSession } = await import('./contribute-check.js'); const { formatStopHookOutput, relayWhenHidden } = await import('./utils/hook-output.js'); @@ -331,7 +307,8 @@ const pendingHintHandler: HookHandler = { // Always consume the stash so a hint stashed before the team turned the // feature off is not delivered later when it is turned back on. const stashed = await pending.takePendingHint(sessionId); - const hint = (await contributeHintAllowed()) ? stashed : null; + const { contributeHintAllowed } = await import('./skill-content.js'); + const hint = (await contributeHintAllowed(resolveHookCwd(stdin))) ? stashed : null; const votesHint = await pending.takePendingVotesHint(sessionId); // The votes nudge instructs the model; the contribute hint asks it to relay diff --git a/src/init.ts b/src/init.ts index 14a6f467..befa082a 100644 --- a/src/init.ts +++ b/src/init.ts @@ -1662,7 +1662,7 @@ export async function init(options: GlobalOptions & { if (stubDeployed > 0) { log.info('The built-in teamai skill is ready in your IDE; it loads its workflows with `teamai skill get`.'); } else { - log.warn('The built-in teamai skill was not deployed to any AI tool (see the lines above); run `teamai pull` once the cause is fixed, or `teamai doctor` to see it.'); + log.warn('The built-in teamai skill was not deployed to any AI tool, so agents cannot find TeamAI yet. The reason is printed above or recorded in ~/.teamai/debug.log; the usual one is that none of the selected tools is installed. Run `teamai pull` once it is fixed.'); } log.info('Skills, rules, env and docs auto-sync on each session start when the selected agent has active TeamAI hooks.'); log.info('Run `teamai status` to check current config.'); diff --git a/src/pull.ts b/src/pull.ts index 4eb55349..3b2d0325 100644 --- a/src/pull.ts +++ b/src/pull.ts @@ -730,6 +730,17 @@ async function reconcileEnvForUnchangedRepo( } } +/** + * The stub is the agent's only way into TeamAI, so a failure to deploy it is + * not silent. A SessionStart pull runs detached with its output discarded, so + * debug.log keeps the record. + */ +function warnStubNotDeployed(scopeLabel: string, e: unknown): void { + const message = `[${scopeLabel}] The built-in teamai skill was not deployed: ${e instanceof Error ? e.message : String(e)}`; + log.warn(message); + log.persist(message); +} + async function pullForScope( localConfig: LocalConfig, options: GlobalOptions, @@ -1015,7 +1026,12 @@ async function pullForScope( const skipRecall = !isRecallEnabled(localConfig, freshConfig); try { const { deployBuiltinAgents } = await import('./builtin-agents.js'); await deployBuiltinAgents(freshConfig, localConfig, { skipRecall }); } catch {} try { const { deployBuiltinRules } = await import('./builtin-rules.js'); await deployBuiltinRules(freshConfig, localConfig, { skipRecall }); } catch {} - try { const { deployBuiltinSkills } = await import('./builtin-skills.js'); await deployBuiltinSkills(freshConfig, localConfig); } catch {} + try { + const { deployBuiltinSkills } = await import('./builtin-skills.js'); + await deployBuiltinSkills(freshConfig, localConfig); + } catch (e) { + warnStubNotDeployed(scopeLabel, e); + } // Refresh managed culture/shared-instruction blocks as well. A CLI // upgrade may add a new target file while the team repo SHA and tool // target set remain unchanged. @@ -1294,7 +1310,7 @@ async function pullForScope( log.debug(`[${scopeLabel}] Deployed ${deployed} built-in skill(s)`); } } catch (e) { - log.debug(`[${scopeLabel}] Built-in skills deployment skipped: ${(e as Error).message}`); + warnStubNotDeployed(scopeLabel, e); } } diff --git a/src/skill-cmd.ts b/src/skill-cmd.ts index fe6786dd..e8c69ace 100644 --- a/src/skill-cmd.ts +++ b/src/skill-cmd.ts @@ -1,5 +1,4 @@ import path from 'node:path'; -import { autoDetectInit, NotInitializedError } from './config.js'; import { log } from './utils/logger.js'; import { listDirs, pathExists } from './utils/fs.js'; import { SkillsHandler } from './resources/skills.js'; @@ -14,8 +13,16 @@ import { } from './agent-skills.js'; import { detectInstalledAgents, type ResolvedAgent } from './known-agents.js'; import { LEGACY_BUILTIN_SKILL_NAMES } from './builtin-skills.js'; -import { blockMessage, resolveServableSkill, skillCatalog, type SkillBlockReason } from './skill-content.js'; -import type { GlobalOptions, LocalConfig, TeamaiConfig } from './types.js'; +import { + BLOCK_NOTES, + detectTeam, + refuseBlocked, + resolveServableSkill, + skillCatalog, + type BlockedSkill, + type ServableSkillResolution, +} from './skill-content.js'; +import type { GlobalOptions, LocalConfig } from './types.js'; const DESCRIPTION_MAX = 160; @@ -30,13 +37,6 @@ interface ResolvedSkill { namespace?: string; } -/** A packaged skill the serving gate withholds; there is no path to print. */ -interface BlockedSkill { - kind: 'blocked'; - name: string; - reason: SkillBlockReason; -} - type LocatedSkill = ResolvedSkill | BlockedSkill; /** @@ -49,45 +49,50 @@ type LocatedSkill = ResolvedSkill | BlockedSkill; * we print under "Repo path" or "Installed in". */ export async function skillShow(name: string, options: GlobalOptions): Promise { - let init: { localConfig: LocalConfig; teamConfig: TeamaiConfig }; - try { - init = await autoDetectInit(); - } catch (e) { - // A packaged skill needs no team: it ships with the CLI, so `teamai skill - // show core` still works on a machine that has never run `teamai init`. - // Only that case: a broken config is reported, not read as "no team". - if (!(e instanceof NotInitializedError)) throw e; - const packaged = await resolveServableSkill(name); - if (packaged.kind === 'blocked') { - const { headline, hint } = blockMessage(packaged.name, packaged.reason); - log.error(headline); - log.dim(hint); - process.exitCode = 1; + // One config load for both the gate and the lookup, so a broken config is + // reported once. + const team = await detectTeam(); + const served = await resolveServableSkill(name, undefined, team); + if (team.kind !== 'team') { + // Only the package can answer without a team: it ships with the CLI, so + // `teamai skill show core` still works on a machine that has never run + // `teamai init`. A config that cannot be loaded leaves the team unknown + // too, so the repo and agents detection would fall back to (another + // team's) are not searched, and the member is told what failed. + if (served.kind === 'blocked') { + refuseBlocked(served); return; } - if (packaged.kind !== 'found') { + if (served.kind !== 'found') { log.error(`Skill "${name}" not found among the skills the installed CLI serves.`); - log.dim('Run `teamai init` first to search the team repo and installed agents too.'); + log.dim(team.kind === 'none' + ? 'Run `teamai init` first to search the team repo and installed agents too.' + : `The team repo and installed agents were not searched: the teamai config could not be loaded. ${team.detail}`); process.exitCode = 1; return; } printSkillCard({ - name: packaged.skill.name, + name: served.skill.name, source: { kind: 'builtin' }, - description: truncate(await readSkillDescription(path.join(packaged.skill.dir, 'SKILL.md')), DESCRIPTION_MAX), + description: truncate(await readSkillDescription(path.join(served.skill.dir, 'SKILL.md')), DESCRIPTION_MAX), contributors: [], tags: [], - primaryPath: packaged.skill.dir, + primaryPath: served.skill.dir, primaryOrigin: 'builtin', installedIn: [], }); - log.dim('No team is set up on this machine, so contributors, tags and installed agents are not shown.'); + if (team.kind === 'none') { + log.dim('No team is set up on this machine, so contributors, tags and installed agents are not shown.'); + } else { + log.error(`The teamai config could not be loaded, so contributors, tags and installed agents are not shown. ${team.detail}`); + process.exitCode = 1; + } return; } - const { localConfig, teamConfig } = init; + const { localConfig, teamConfig } = team.init; const agents = await detectInstalledAgents(localConfig, teamConfig); - const located = await locateSkill(name, localConfig, agents); + const located = await locateSkill(name, localConfig, agents, served); if (!located) { log.error(`Skill "${name}" not found in team repo or any installed agent.`); log.dim('Try `teamai list --source all` to see available skills.'); @@ -97,10 +102,7 @@ export async function skillShow(name: string, options: GlobalOptions): Promise { - const catalog = await skillCatalog(); + // One config load for the catalog's gate and the team listing, so a broken + // config is reported once. + const team = await detectTeam(); + const catalog = await skillCatalog(undefined, team); if (options.json) { console.log(JSON.stringify({ skills: catalog }, null, 2)); @@ -153,19 +158,18 @@ export async function skillList(options: GlobalOptions & { json?: boolean }): Pr // The packaged catalog needs no team: a machine that has not run `teamai init` // still gets to discover what the installed CLI serves, like `skill get` does. - let initialized = true; - try { - await autoDetectInit(); - } catch (e) { - if (!(e instanceof NotInitializedError)) throw e; - initialized = false; - } - if (initialized) { + // A config that cannot be loaded lists no team either, rather than the one + // detection would fall back to, and says what failed. + if (team.kind === 'team') { const { list } = await import('./status.js'); await list('skills', { ...options, source: 'all' }); - } else { + } else if (team.kind === 'none') { log.dim('Not initialized: run `teamai init` to list team and installed skills.'); console.log(''); + } else { + log.error(`Team and installed skills are not listed: the teamai config could not be loaded. ${team.detail}`); + console.log(''); + process.exitCode = 1; } console.log('=== BUILT-IN SKILLS (served by the CLI) ==='); @@ -174,10 +178,7 @@ export async function skillList(options: GlobalOptions & { json?: boolean }): Pr console.log(' (none — the installed package ships no skill content)'); } else { for (const entry of catalog) { - const note = entry.blockedBy === 'recall' ? ' (needs recall — teamai recall enable)' - : entry.blockedBy === 'read-only' ? ' (not available on a read-only HTTP source)' - : entry.blockedBy === 'config' ? ' (not available: the teamai config could not be loaded)' : ''; - console.log(` ${entry.name}${note}`); + console.log(` ${entry.name}${entry.blockedBy ? ` (${BLOCK_NOTES[entry.blockedBy]})` : ''}`); console.log(` ${truncate(entry.description, DESCRIPTION_MAX) || '(no description)'}`); console.log(` teamai skill get ${entry.name}`); } @@ -189,6 +190,7 @@ async function locateSkill( name: string, localConfig: LocalConfig, agents: ResolvedAgent[], + served: ServableSkillResolution, ): Promise { const teamSkillsDir = path.join(localConfig.repo.localPath, 'skills'); @@ -227,8 +229,7 @@ async function locateSkill( // 4. Built-in skill served by the CLI, including legacy-name aliases. Last, // so it answers for the names nothing on this machine claims: `core` and // `wiki` live in the package, and the agent directory holds only the stub. - const served = await resolveServableSkill(name); - if (served.kind === 'blocked') return { kind: 'blocked', name: served.name, reason: served.reason }; + if (served.kind === 'blocked') return served; if (served.kind === 'found') { return { kind: 'found', name: served.skill.name, primaryPath: served.skill.dir, primaryOrigin: 'builtin' }; } diff --git a/src/skill-content.ts b/src/skill-content.ts index 99569b90..31e4c242 100644 --- a/src/skill-content.ts +++ b/src/skill-content.ts @@ -4,7 +4,8 @@ import { fileURLToPath } from 'node:url'; import chalk from 'chalk'; import { listFilesRecursive, pathExists } from './utils/fs.js'; import { readSkillDescription } from './agent-skills.js'; -import { setStderrOnly } from './utils/logger.js'; +import { log, setStderrOnly } from './utils/logger.js'; +import type { TeamaiInit } from './config.js'; // ─── CLI-served skill content ──────────────────────────── // @@ -62,46 +63,144 @@ const SKILL_ALIASES: Readonly> = { */ const RECALL_DEPENDENT_SKILLS = new Set(['share']); -/** Why a served skill is withheld right now. */ -export type SkillBlockReason = 'recall' | 'read-only' | 'config'; +/** + * Why a served skill is withheld right now. A config that cannot be loaded + * carries what failed: nothing else reports it, because detection skips a + * broken project file and `teamai doctor` reads the one it falls back to. + */ +export type SkillBlock = + | { reason: 'recall' } + | { reason: 'read-only' } + | { reason: 'config'; detail: string }; + +export type SkillBlockReason = SkillBlock['reason']; + +/** + * The share gate's answer. A blocked answer carries no config: nothing past the + * gate may act on it. An open one carries the config it was decided on, or + * null when there is none on the machine. + */ +export type ShareGate = + | { block: SkillBlock; config: null } + | { block: null; config: TeamaiInit | null }; + +/** + * Which team this directory belongs to, or why that is unknown. `share`, and + * the `skill show` / `skill list` lookups, ask this so none of them answers for + * the wrong team: detection skips a broken project config and falls back to + * the user config, another team's repo, recall and source. A config that + * cannot be loaded carries what failed, since nothing else reports it. Only + * loading the config is read as "cannot be loaded". + */ +export type TeamDetection = + | { kind: 'team'; init: TeamaiInit } + | { kind: 'none' } + | { kind: 'unusable'; detail: string }; + +export async function detectTeam(cwd?: string): Promise { + const { autoDetectInit, findUnreadableProjectConfig, requireInit, NotInitializedError, BROKEN_CONFIG_ADVICE } = + await import('./config.js'); + // Loading the config can migrate it and say so with `log.info`. That line + // must not land in the skill content, the JSON these commands print on + // stdout, or a hook's reply, so config loading reports on stderr here. + const previous = setStderrOnly(true); + try { + if (cwd !== undefined) { + try { + await fs.promises.stat(cwd); + } catch (e) { + // A directory that no longer exists (a hook payload naming a deleted + // worktree) holds no project config, and git refuses to open it. Any + // other failure (no permission, a path through a file) leaves the + // project unknown, not absent. + if (typeof e === 'object' && e !== null && 'code' in e && e.code === 'ENOENT') { + return { kind: 'team', init: await requireInit() }; + } + return { kind: 'unusable', detail: `${cwd} cannot be checked: ${e instanceof Error ? e.message : String(e)}` }; + } + } + const unreadable = await findUnreadableProjectConfig(cwd); + if (unreadable) { + // A parse error spans several lines (a code frame); its first names the + // file, the line and the column, which is what the member acts on. + return { kind: 'unusable', detail: `${firstLine(unreadable)}. ${BROKEN_CONFIG_ADVICE}` }; + } + return { kind: 'team', init: await autoDetectInit(cwd) }; + } catch (e) { + if (e instanceof NotInitializedError) return { kind: 'none' }; + return { kind: 'unusable', detail: firstLine(e instanceof Error ? e.message : String(e)) }; + } finally { + setStderrOnly(previous); + } +} /** - * What makes this skill unusable right now, or null. + * Whether `share` can be served here. The Stop-hook reminder asks this too, so + * the nudge and `teamai skill get share` cannot disagree, and it reads its own + * on/off switch from the config returned instead of loading it a second time. * * Fails open only where there is no config at all: a fresh install reading * the docs gets the content rather than a refusal it cannot act on. A config * that exists but cannot be loaded blocks: whether recall is on, or the source * writable, is then unknown, and the workflow would fail at `teamai contribute`. + * Any failure past loading the config is a fault here and propagates. */ -async function blockReason(name: string): Promise { - if (!RECALL_DEPENDENT_SKILLS.has(name)) return null; - const { NotInitializedError } = await import('./config.js'); +export async function shareGate(cwd?: string): Promise { + return gateFor(await detectTeam(cwd)); +} + +/** The share gate on a team already detected, for a command that needs the team too. */ +async function gateFor(team: TeamDetection): Promise { + const { isRecallEnabled } = await import('./types.js'); + if (team.kind === 'none') return { block: null, config: null }; + if (team.kind === 'unusable') return { block: { reason: 'config', detail: team.detail }, config: null }; + const { localConfig, teamConfig } = team.init; + // `teamai contribute` refuses a read-only source (read-only.ts), so the + // workflow would fail at its last step after the agent did all the work. + if (localConfig.repo?.kind === 'http') return { block: { reason: 'read-only' }, config: null }; + if (!isRecallEnabled(localConfig, teamConfig)) return { block: { reason: 'recall' }, config: null }; + return { block: null, config: team.init }; +} + +/** The first line of an error, without the colon that introduces its code frame. */ +function firstLine(text: string): string { + return text.trim().split('\n')[0].trim().replace(/:$/, ''); +} + +/** + * Whether the Stop-hook share reminder may be shown. Resolved per hook run so + * a team can switch it off in teamai.yaml (or a member in local config) without + * re-injecting hooks; on by default. Never with no config at all: a project + * that never set up teamai has no team to share with, although + * `teamai skill get share` still serves there. + * + * The reminder routes to `share`, so it is withheld wherever `shareGate` + * blocks it: a nudge there would send the agent to a command that says no. + * Both the hook dispatcher and the legacy `teamai contribute-check` ask this, + * passing the session's cwd: the process may run anywhere, and a `chdir` into + * the cwd can fail. + */ +export async function contributeHintAllowed(cwd?: string): Promise { + const { isContributeHintEnabled } = await import('./types.js'); + let gate: ShareGate; try { - const [{ autoDetectInit, findUnreadableProjectConfig }, { isRecallEnabled }] = await Promise.all([ - import('./config.js'), - import('./types.js'), - ]); - // Loading the config can migrate it and say so with `log.info`. That line - // must not land in the skill content or the JSON these commands print on - // stdout, so config loading reports on stderr for this one call. - const previous = setStderrOnly(true); - let loaded: Awaited>; - try { - // A broken project config is skipped by detection, which would then - // answer with the user config: another team's recall and source. - if (await findUnreadableProjectConfig()) return 'config'; - loaded = await autoDetectInit(); - } finally { - setStderrOnly(previous); - } - const { localConfig, teamConfig } = loaded; - // `teamai contribute` refuses a read-only source (read-only.ts), so the - // workflow would fail at its last step after the agent did all the work. - if (localConfig.repo?.kind === 'http') return 'read-only'; - return isRecallEnabled(localConfig, teamConfig) ? null : 'recall'; + gate = await shareGate(cwd); } catch (e) { - return e instanceof NotInitializedError ? null : 'config'; + // A fault in the gate itself, not a config it could not load (the gate + // answers that): a Stop hook must not fail the turn over a reminder. + log.debug(`share gate failed, reminder withheld: ${e instanceof Error ? e.message : String(e)}`); + return false; } + if (gate.block) return false; + return gate.config + ? isContributeHintEnabled(gate.config.localConfig, gate.config.teamConfig) + : false; +} + +/** What makes this skill unusable right now, or null. */ +async function blockReason(name: string, team?: TeamDetection): Promise { + if (!RECALL_DEPENDENT_SKILLS.has(name)) return null; + return (team ? await gateFor(team) : await shareGate()).block; } /** A skill directory that ships inside the npm package. */ @@ -196,9 +295,12 @@ async function resolvePackagedSkill( */ export type ServableSkillResolution = | { kind: 'found'; skill: PackagedSkill } - | { kind: 'blocked'; name: string; reason: SkillBlockReason } + | BlockedSkill | { kind: 'not-found'; name: string }; +/** A packaged skill the gate withholds: its canonical name and why. */ +export type BlockedSkill = { kind: 'blocked'; name: string } & SkillBlock; + /** * The only way to obtain a packaged skill outside this module. * @@ -209,17 +311,25 @@ export type ServableSkillResolution = export async function resolveServableSkill( name: string, roots: PackagedSkillRoots = packagedSkillRoots(), + team?: TeamDetection, ): Promise { const skill = await resolvePackagedSkill(name, roots); if (!skill) return { kind: 'not-found', name }; - const reason = await blockReason(skill.name); - if (reason) return { kind: 'blocked', name: skill.name, reason }; + const block = await blockReason(skill.name, team); + if (block) return { kind: 'blocked', name: skill.name, ...block }; return { kind: 'found', skill }; } +/** The short note `skill list` prints beside a blocked skill, per reason. */ +export const BLOCK_NOTES: Record = { + recall: 'needs recall — teamai recall enable', + 'read-only': 'not available on a read-only HTTP source', + config: 'not available: the teamai config could not be loaded', +}; + /** The two lines every command prints for a blocked skill. */ -export function blockMessage(name: string, reason: SkillBlockReason): { headline: string; hint: string } { - switch (reason) { +export function blockMessage(name: string, block: SkillBlock): { headline: string; hint: string } { + switch (block.reason) { case 'recall': return { headline: `${name} needs recall, which is disabled for this team.`, @@ -233,10 +343,10 @@ export function blockMessage(name: string, reason: SkillBlockReason): { headline case 'config': return { headline: `${name} is not available: the teamai config on this machine could not be loaded, so whether it can contribute is unknown.`, - hint: 'Run `teamai doctor` to see what is wrong with it, then try again.', + hint: block.detail, }; default: { - const exhaustive: never = reason; + const exhaustive: never = block; throw new Error(`Unhandled block reason ${String(exhaustive)}`); } } @@ -313,8 +423,8 @@ function rootsMissing(): void { * cannot finish whichever way it asks. A routing aid, not access control: the * files ship in the npm package either way. */ -function refuseBlocked(name: string, reason: SkillBlockReason): void { - const { headline, hint } = blockMessage(name, reason); +export function refuseBlocked(blocked: BlockedSkill): void { + const { headline, hint } = blockMessage(blocked.name, blocked); diagnostic(`${chalk.red('✖')} ${headline}`); diagnostic(` ${hint}`); process.exitCode = 1; @@ -360,7 +470,7 @@ export async function skillGet(names: string[], options: SkillGetOptions = {}): // either found or blocked. if (resolved.kind !== 'found') { if (resolved.kind === 'blocked') { - const { headline, hint } = blockMessage(listed.name, resolved.reason); + const { headline, hint } = blockMessage(listed.name, resolved); diagnostic(`${chalk.yellow('⚠')} Skipped ${listed.name}. ${headline} ${hint}`); } continue; @@ -375,7 +485,7 @@ export async function skillGet(names: string[], options: SkillGetOptions = {}): return; } if (resolved.kind === 'blocked') { - refuseBlocked(resolved.name, resolved.reason); + refuseBlocked(resolved); return; } targets.push(resolved.skill); @@ -412,7 +522,7 @@ export async function skillPath(name: string): Promise { notFound(name, await listServableSkills(roots)); return; case 'blocked': - refuseBlocked(resolved.name, resolved.reason); + refuseBlocked(resolved); return; case 'found': console.log(resolved.skill.dir); @@ -445,11 +555,14 @@ export type SkillCatalogEntry = | (SkillCatalogEntryFields & { blockedBy: null; path: string }) | (SkillCatalogEntryFields & { blockedBy: SkillBlockReason; path: null }); -export async function skillCatalog(roots: PackagedSkillRoots = packagedSkillRoots()): Promise { +export async function skillCatalog( + roots: PackagedSkillRoots = packagedSkillRoots(), + team?: TeamDetection, +): Promise { const skills = await listServableSkills(roots); const entries: SkillCatalogEntry[] = []; for (const skill of skills) { - const resolved = await resolveServableSkill(skill.name, roots); + const resolved = await resolveServableSkill(skill.name, roots, team); const fields: SkillCatalogEntryFields = { name: skill.name, description: await readSkillDescription(path.join(skill.dir, SKILL_MD)), diff --git a/src/utils/logger.ts b/src/utils/logger.ts index aba89b09..38cc4a3c 100644 --- a/src/utils/logger.ts +++ b/src/utils/logger.ts @@ -10,7 +10,7 @@ let stderrMode = false; // ─── File transport ───────────────────────────────────── // -// All log.debug() and log.error() calls are persisted to +// All log.debug(), log.error() and log.persist() calls are persisted to // ~/.teamai/debug.log via synchronous append. This ensures // hook processes (short-lived, stdout swallowed by Claude Code) // leave a durable trace for troubleshooting. @@ -146,6 +146,14 @@ export const log = { if (silentMode) return; writeInfoLine(`${chalk.yellow('⚠')} ${msg}`); }, + /** + * Record a failure in debug.log only, never on the console: for a warning + * already shown where a detached hook process (stdout and stderr discarded, + * silent) would lose it, without printing it twice under --verbose. + */ + persist(msg: string): void { + writeToFile('WARN', msg); + }, error(msg: string): void { console.error(chalk.red('✖'), msg); writeToFile('ERROR', msg);