From 4242918630faf50fc3145eedb8c9ec815541d9bd Mon Sep 17 00:00:00 2001 From: Kevin Cui Date: Wed, 29 Jul 2026 06:30:15 -0400 Subject: [PATCH 1/2] feat(skills): add auto-trigger control for bundled skills Users reported that the bundled skills fire on their own whenever a prompt happens to match their description, and asked to be able to require naming them explicitly. No two agents spell that policy the same way, so the effective value is written into every published copy as both vendor markers, always explicit and always inverse of each other: `disable-model-invocation` in the SKILL.md frontmatter for Claude Code, and `policy.allow_implicit_invocation` in `agents/openai.yaml` for Codex. Both are written for every agent rather than gated on a per-agent capability table: the Agent Skills standard requires runtimes to ignore markers they do not recognize, and the behavior of the eight Claude Code derivatives could not be verified either way. Only directories oo already generates are touched. Codex's `config.toml` and Claude's `skillOverrides` would work too, but both are hand-edited and concurrently written by those tools' own UIs. See ADR 0002. The policy is an input to publication rather than a runtime switch, so `off`/`on` republish immediately and startup synchronization keeps its cheap short-circuit; `oo skills repair` applies a hand-edited value. `--all` is a standing policy that keeps covering bundled skills added by later releases, and `--out-dir` exports keep the shipped default so a vendored copy never carries one machine's preference. Signed-off-by: Kevin Cui --- README-ZH_CN.md | 4 + README.md | 5 + .../skills/shared/oo-create-skill/SKILL.md | 1 + .../shared/oo-create-skill/agents/openai.yaml | 5 + contrib/skills/shared/oo-find-skills/SKILL.md | 1 + .../shared/oo-find-skills/agents/openai.yaml | 5 + .../skills/shared/oo-publish-skill/SKILL.md | 1 + .../oo-publish-skill/agents/openai.yaml | 5 + contrib/skills/shared/oo/SKILL.md | 1 + contrib/skills/shared/oo/agents/openai.yaml | 5 + docs/commands.md | 105 +++ docs/commands.zh-CN.md | 91 +++ src/application/commands/skills/auto-sync.ts | 13 +- .../skills/auto-trigger-policy.test.ts | 145 ++++ .../commands/skills/auto-trigger-policy.ts | 81 +++ .../skills/auto-trigger/index.cli.test.ts | 673 ++++++++++++++++++ .../commands/skills/auto-trigger/index.ts | 16 + .../commands/skills/auto-trigger/off.ts | 13 + .../commands/skills/auto-trigger/on.ts | 15 + .../commands/skills/auto-trigger/publish.ts | 115 +++ .../commands/skills/auto-trigger/report.ts | 50 ++ .../commands/skills/auto-trigger/status.ts | 73 ++ .../skills/auto-trigger/toggle-command.ts | 226 ++++++ .../commands/skills/embedded-assets.test.ts | 31 + .../commands/skills/embedded-assets.ts | 63 +- src/application/commands/skills/index.test.ts | 99 +-- src/application/commands/skills/index.ts | 2 + .../commands/skills/install.cli.test.ts | 27 + src/application/commands/skills/repair.ts | 11 +- src/application/commands/skills/shared.ts | 19 +- .../commands/telemetry-decisions.test.ts | 28 + src/application/schemas/settings.test.ts | 122 ++++ src/application/schemas/settings.ts | 161 ++++- src/i18n/catalog.ts | 104 +++ 34 files changed, 2244 insertions(+), 72 deletions(-) create mode 100644 contrib/skills/shared/oo-create-skill/agents/openai.yaml create mode 100644 contrib/skills/shared/oo-find-skills/agents/openai.yaml create mode 100644 contrib/skills/shared/oo-publish-skill/agents/openai.yaml create mode 100644 contrib/skills/shared/oo/agents/openai.yaml create mode 100644 src/application/commands/skills/auto-trigger-policy.test.ts create mode 100644 src/application/commands/skills/auto-trigger-policy.ts create mode 100644 src/application/commands/skills/auto-trigger/index.cli.test.ts create mode 100644 src/application/commands/skills/auto-trigger/index.ts create mode 100644 src/application/commands/skills/auto-trigger/off.ts create mode 100644 src/application/commands/skills/auto-trigger/on.ts create mode 100644 src/application/commands/skills/auto-trigger/publish.ts create mode 100644 src/application/commands/skills/auto-trigger/report.ts create mode 100644 src/application/commands/skills/auto-trigger/status.ts create mode 100644 src/application/commands/skills/auto-trigger/toggle-command.ts diff --git a/README-ZH_CN.md b/README-ZH_CN.md index ca975d8..62ff018 100644 --- a/README-ZH_CN.md +++ b/README-ZH_CN.md @@ -81,6 +81,10 @@ QoderWork、DeepSeek TUI。 内置 skill 会随 `oo` 每次发布同步更新。具体的 skill 安装位置和手动管理方式见 [命令参考](./docs/commands.zh-CN.md)。 +当请求与内置 skill 匹配时，Agent 可能会自行使用它。若希望必须由你点名后才触发， +运行 `oo skills auto-trigger off --all`（也可只指定其中几个）。skill 仍然安装 +着、仍可调用——在 Claude Code 中用 `/oo`，在 Codex 中用 `$oo`。 + ## 隐私 `oo` 默认记录受隐私约束的 telemetry，事件不包含自由文本输入、路径、用户名、 diff --git a/README.md b/README.md index e3ebcea..6a1d2fb 100644 --- a/README.md +++ b/README.md @@ -88,6 +88,11 @@ Bundled skills are kept in sync with each `oo` release. See the [command reference](./docs/commands.md) for the exact skill targets and how to manage them manually. +Agents may use a bundled skill on their own when a request matches it. To make +them wait until you ask by name, run `oo skills auto-trigger off --all` (or name +individual skills). The skills stay installed and invocable — `/oo` in Claude +Code, `$oo` in Codex. + ## Privacy `oo` records privacy-constrained telemetry by default. Events do not include diff --git a/contrib/skills/shared/oo-create-skill/SKILL.md b/contrib/skills/shared/oo-create-skill/SKILL.md index e6efc3e..8e8d319 100644 --- a/contrib/skills/shared/oo-create-skill/SKILL.md +++ b/contrib/skills/shared/oo-create-skill/SKILL.md @@ -1,6 +1,7 @@ --- name: oo-create-skill description: Create, adopt, review, or update local AI agent skills, including ordinary knowledge or workflow skills and skills powered by oo connectors or hosted capabilities. Use when the user asks to create or improve a skill, turn existing files or scripts into a skill, check a skill against modern authoring practices, or build a reusable skill that calls oo at runtime. +disable-model-invocation: allowed-tools: [Bash(oo *)] diff --git a/contrib/skills/shared/oo-create-skill/agents/openai.yaml b/contrib/skills/shared/oo-create-skill/agents/openai.yaml new file mode 100644 index 0000000..5295682 --- /dev/null +++ b/contrib/skills/shared/oo-create-skill/agents/openai.yaml @@ -0,0 +1,5 @@ +# Managed by oo. Change this through `oo skills auto-trigger`, not by hand. +# `allow_implicit_invocation: false` stops Codex from loading this skill from a +# user prompt; explicit `$oo-create-skill` invocation keeps working. +policy: + allow_implicit_invocation: diff --git a/contrib/skills/shared/oo-find-skills/SKILL.md b/contrib/skills/shared/oo-find-skills/SKILL.md index 409618f..336705b 100644 --- a/contrib/skills/shared/oo-find-skills/SKILL.md +++ b/contrib/skills/shared/oo-find-skills/SKILL.md @@ -1,6 +1,7 @@ --- name: oo-find-skills description: Find, compare, and install published OOMOL/oo skills. Use when the user asks to find, search for, discover, recommend, compare, choose, or install an existing skill for a task; asks whether there is a skill that can do something; or explicitly mentions the OOMOL/oo skill catalog. Do not use for creating or editing local skills, generic skill design, or non-OOMOL skill catalogs. +disable-model-invocation: allowed-tools: [Bash(oo *)] diff --git a/contrib/skills/shared/oo-find-skills/agents/openai.yaml b/contrib/skills/shared/oo-find-skills/agents/openai.yaml new file mode 100644 index 0000000..c81f550 --- /dev/null +++ b/contrib/skills/shared/oo-find-skills/agents/openai.yaml @@ -0,0 +1,5 @@ +# Managed by oo. Change this through `oo skills auto-trigger`, not by hand. +# `allow_implicit_invocation: false` stops Codex from loading this skill from a +# user prompt; explicit `$oo-find-skills` invocation keeps working. +policy: + allow_implicit_invocation: diff --git a/contrib/skills/shared/oo-publish-skill/SKILL.md b/contrib/skills/shared/oo-publish-skill/SKILL.md index 755d363..6d26edc 100644 --- a/contrib/skills/shared/oo-publish-skill/SKILL.md +++ b/contrib/skills/shared/oo-publish-skill/SKILL.md @@ -1,6 +1,7 @@ --- name: oo-publish-skill description: Publish, release, upload, or submit an existing AI agent skill directory with SKILL.md to the OOMOL registry by running oo skills publish, or generate a share prompt for a published skill by running oo skills share, including temporary shares for private packages. Use when the user asks to publish a skill, share a published skill, make a skill available in the OOMOL skill catalog, release a registry skill package, resolve publish visibility, version, package-name, or overwrite prompts, or publish from a local, registry-installed, or path-based skill source. Do not use for finding, installing, creating, or editing skills unless the final goal is publication or sharing. +disable-model-invocation: allowed-tools: [Bash(oo *)] diff --git a/contrib/skills/shared/oo-publish-skill/agents/openai.yaml b/contrib/skills/shared/oo-publish-skill/agents/openai.yaml new file mode 100644 index 0000000..2bebff9 --- /dev/null +++ b/contrib/skills/shared/oo-publish-skill/agents/openai.yaml @@ -0,0 +1,5 @@ +# Managed by oo. Change this through `oo skills auto-trigger`, not by hand. +# `allow_implicit_invocation: false` stops Codex from loading this skill from a +# user prompt; explicit `$oo-publish-skill` invocation keeps working. +policy: + allow_implicit_invocation: diff --git a/contrib/skills/shared/oo/SKILL.md b/contrib/skills/shared/oo/SKILL.md index ce1ad64..11a171b 100644 --- a/contrib/skills/shared/oo/SKILL.md +++ b/contrib/skills/shared/oo/SKILL.md @@ -1,6 +1,7 @@ --- name: oo description: Use OO for connected accounts, APIs, and hosted AI tasks; the first-choice router for tasks whose outcome lives outside this workspace, including connected third-party accounts (email, calendar, drive, chat, notes, issue tracker, code host, CRM, storage, etc.), an external API, or a managed AI pipeline (OCR, translation, transcription, TTS, text-to-image, subtitles, long-document understanding). Use when local code needs OOMOL LLM client configuration such as an OpenAI-compatible base URL, API key, or model name. Otherwise use only when the user wants an existing hosted capability or connector workflow, not a local implementation. Concrete capabilities are discovered at runtime, so no package, block, connector, or action names are assumed in advance. Match intent across languages. Skip other pure local coding, shell glue, repo edits, and text-only answers an LLM can complete without hosted capability execution. +disable-model-invocation: allowed-tools: [Bash(oo *)] diff --git a/contrib/skills/shared/oo/agents/openai.yaml b/contrib/skills/shared/oo/agents/openai.yaml new file mode 100644 index 0000000..37fbd03 --- /dev/null +++ b/contrib/skills/shared/oo/agents/openai.yaml @@ -0,0 +1,5 @@ +# Managed by oo. Change this through `oo skills auto-trigger`, not by hand. +# `allow_implicit_invocation: false` stops Codex from loading this skill from a +# user prompt; explicit `$oo` invocation keeps working. +policy: + allow_implicit_invocation: diff --git a/docs/commands.md b/docs/commands.md index 9cc01b4..54d50bc 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -1934,6 +1934,111 @@ Common rules: - The JSON payload never includes `apiKey`, raw HTTP request/response bodies, stack traces, or unredacted endpoint secrets. +### `oo skills auto-trigger` + +Control whether agents may load a bundled skill without being asked to. This is +a command group with three subcommands, and it applies only to the four bundled +skills (`oo`, `oo-find-skills`, `oo-create-skill`, `oo-publish-skill`). Registry +and local skills are unaffected. + +Turning auto-trigger off leaves the skill installed and still invocable by name +— `/oo` in Claude Code and Claude-compatible agents, `$oo` in Codex. What +changes is that the agent no longer offers to use it on its own. + +- Scope: the setting is per skill, not per agent. One run applies it to every + supported agent host, using whichever mechanism that agent understands. +- Persistence: the choice is stored in the CLI settings file under + `[skills.auto_trigger]` and survives across sessions. +- Application: the setting is an input to skill publication, not a runtime + switch. `off`/`on` republish the bundled skills immediately. Startup + synchronization does not detect a hand-edited `[skills.auto_trigger]`; apply + one with `oo skills repair --skill oo --skill oo-find-skills --skill + oo-create-skill --skill oo-publish-skill` (`--skill` is required). +- `--all` is a standing policy, not a snapshot: bundled skills added by later + releases are covered by it without a further command. While it is in force, a + per-skill `off`/`on` still updates the stored list but changes nothing an + agent can see, and the text output says so. +- Consequence: with auto-trigger off, an agent that hides a skill's description + from its own context can no longer act on it unprompted. The bundled `oo` + skill's end-of-session suggestions are one such behavior and stop happening + until auto-trigger is turned back on. + +#### `oo skills auto-trigger off [skillName...]` + +Make bundled skills manual-only. + +- Arguments: `[skillName...]` bundled skill names to make manual-only; they are + added to a persisted list, de-duplicated and sorted. +- Options: `--all` makes every bundled skill manual-only, now and in future + releases, and clears the per-skill list because it supersedes it. + `--json` / `--format json` / `--show-schema-version` control output. +- Validation: pass skill names **or** `--all`, not both and not neither; either + misuse exits `2`. A name that is not a bundled skill exits `2` and lists the + accepted names. +- Safety: a same-name skill directory that `oo` does not manage is left + untouched and reported as skipped. This differs from `oo skills install`, + which fails the run on such a directory: changing a preference must still + reach the other agents. +- Exit code: `1` when the setting was saved but one or more skill targets could + not be republished; the message names them and gives the `oo skills repair` + command to finish applying it. + +#### `oo skills auto-trigger on [skillName...]` + +Let agents load bundled skills on their own again. + +- Arguments: `[skillName...]` bundled skill names to remove from the list. +- Options: `--all` restores the shipped default for every bundled skill, + clearing both the standing policy and the per-skill list, so nothing is left + silently manual-only. Output options match `off`. +- Validation: as `off`, except that `on` also accepts a name the stored + `disabled` list already holds even when it is not a bundled skill in this + release. Removing an entry is the command's job, and a bundled skill dropped + by a later release would otherwise be unclearable except with `--all`. +- Safety and exit codes match `off`. + +#### `oo skills auto-trigger status` + +Show the configured auto-trigger policy for every bundled skill. + +- Options: `--json` / `--format json` / `--show-schema-version`. +- Text output: a headline stating the overall state, then one line per bundled + skill reading `auto`, `manual`, or `manual (all)`. +- Scope: this reports the stored setting, not what is currently published. The + two agree unless the settings file was edited by hand without republishing — + see Application above. + +#### JSON output + +`status` emits the state; `off` and `on` emit the same state plus what they +published. + +```json +{ + "disabled": ["oo-create-skill"], + "disabledAll": false, + "skills": [ + { "autoTrigger": true, "name": "oo", "reason": "default" }, + { "autoTrigger": true, "name": "oo-find-skills", "reason": "default" }, + { "autoTrigger": false, "name": "oo-create-skill", "reason": "skill" }, + { "autoTrigger": true, "name": "oo-publish-skill", "reason": "default" } + ], + "publications": [ + { "agent": "universal", "skill": "oo", "status": "published" }, + { "agent": "claude", "skill": "oo", "status": "skipped" } + ] +} +``` + +- `disabled` echoes the persisted per-skill list verbatim, including a name that + no longer matches a bundled skill in this release. Such an entry has no + effect; clear it with `on `, which accepts a name the list already + holds. +- `reason` is `default` (auto-trigger on), `skill` (this skill was named), or + `all` (the standing policy covers it). `all` outranks `skill`. +- `publications` is present on `off` and `on` only. `status` is one of + `published`, `skipped` (target not managed by `oo`), or `failed`. + ### `oo skills recommend` End-of-session skill suggestions for the bundled `oo` skill, plus controls to diff --git a/docs/commands.zh-CN.md b/docs/commands.zh-CN.md index a49bd51..20ac437 100644 --- a/docs/commands.zh-CN.md +++ b/docs/commands.zh-CN.md @@ -1623,6 +1623,97 @@ CLI 默认记录受隐私约束的命令使用 telemetry。事件不包含 free- - JSON 输出永远不会包含 `apiKey`、原始 HTTP 请求 / 响应体、stack trace 或未 脱敏的 endpoint secret。 +### `oo skills auto-trigger` + +控制 agent 能否在用户未点名的情况下自行加载内置 skill。这是一个命令组，包含三个 +子命令，仅作用于四个内置 skill（`oo`、`oo-find-skills`、`oo-create-skill`、 +`oo-publish-skill`），registry skill 与本地 skill 不受影响。 + +关闭自动触发后，skill 仍然安装着、仍可按名调用——在 Claude Code 及兼容 agent 中 +使用 `/oo`，在 Codex 中使用 `$oo`。改变的只是 agent 不再主动使用它。 + +- 维度：设置按 skill 生效，不按 agent。一次执行即应用到所有受支持的 agent 宿主， + 各自使用该 agent 能识别的机制。 +- 持久化：选择存储在 CLI 设置文件的 `[skills.auto_trigger]` 下，跨会话保留。 +- 生效时机：该设置是 skill 发布时的输入，不是运行时开关。`off`/`on` 会立即重新 + 发布内置 skill。启动同步不会检测手工修改的 `[skills.auto_trigger]`；手改后请用 + `oo skills repair --skill oo --skill oo-find-skills --skill oo-create-skill + --skill oo-publish-skill` 应用（`--skill` 是必填项）。 +- `--all` 是常驻策略而非快照：后续版本新增的内置 skill 也会被它覆盖，无需再次 + 执行命令。该策略生效期间，针对单个 skill 的 `off`/`on` 仍会更新持久化列表，但 + 对 agent 而言没有任何变化，文本输出会明确提示这一点。 +- 连带影响：自动触发关闭后，会把 skill 描述从自身上下文中移除的 agent 将无法再 + 主动使用它。内置 `oo` skill 的收尾阶段推荐即属于此类行为，在重新开启自动触发 + 之前不会再发生。 + +#### `oo skills auto-trigger off [skillName...]` + +把内置 skill 改为仅手动触发。 + +- 参数：`[skillName...]` 要改为仅手动触发的内置 skill 名称；会写入持久化列表， + 去重并排序。 +- 选项：`--all` 把所有内置 skill 改为仅手动触发（含后续版本新增的），并清空 + per-skill 列表，因为它已被覆盖。`--json` / `--format json` / + `--show-schema-version` 控制输出。 +- 校验：传入 skill 名称**或** `--all`，不能同时传、也不能都不传，两种误用均以 + `2` 退出。不属于内置 skill 的名称以 `2` 退出并列出可用名称。 +- 安全性：非 `oo` 管理的同名 skill 目录不会被覆盖，会在输出中报告为已跳过。这与 + `oo skills install` 不同——后者遇到此类目录会让整次运行失败；而修改偏好设置必须 + 仍然能对其他 agent 生效。 +- 退出码：设置已保存但有 skill 目标未能重新发布时退出 `1`，消息会点名这些目标， + 并给出用于完成应用的 `oo skills repair` 命令。 + +#### `oo skills auto-trigger on [skillName...]` + +重新允许 agent 自行加载内置 skill。 + +- 参数：`[skillName...]` 要从列表中移除的内置 skill 名称。 +- 选项：`--all` 把所有内置 skill 恢复为出厂默认，同时清除常驻策略和 per-skill + 列表，不会遗留任何仍处于仅手动状态的 skill。输出选项与 `off` 相同。 +- 校验：与 `off` 相同，但 `on` 额外接受已存在于 `disabled` 列表中的名称，即使它 + 在当前版本中已不是内置 skill。移除条目本就是该命令的职责；否则后续版本删除的 + 内置 skill 将只能用 `--all` 清除。 +- 安全性与退出码同 `off`。 + +#### `oo skills auto-trigger status` + +显示每个内置 skill 已配置的自动触发策略。 + +- 选项：`--json` / `--format json` / `--show-schema-version`。 +- 文本输出：先输出一行整体状态，再为每个内置 skill 输出一行，取值为 `自动`、 + `手动` 或 `手动（全部关闭）`。 +- 范围：显示的是已保存的设置，而非当前已发布到磁盘的内容。除非手改了设置文件却 + 没有重新发布，两者一致——参见上文「生效时机」。 + +#### JSON 输出 + +`status` 输出状态；`off` 与 `on` 输出同样的状态，外加本次发布的结果。 + +```json +{ + "disabled": ["oo-create-skill"], + "disabledAll": false, + "skills": [ + { "autoTrigger": true, "name": "oo", "reason": "default" }, + { "autoTrigger": true, "name": "oo-find-skills", "reason": "default" }, + { "autoTrigger": false, "name": "oo-create-skill", "reason": "skill" }, + { "autoTrigger": true, "name": "oo-publish-skill", "reason": "default" } + ], + "publications": [ + { "agent": "universal", "skill": "oo", "status": "published" }, + { "agent": "claude", "skill": "oo", "status": "skipped" } + ] +} +``` + +- `disabled` 原样回显持久化的 per-skill 列表，包括在当前版本中已不对应任何内置 + skill 的名称。此类条目不产生任何效果；用 `on ` 即可清除——`on` 接受列表 + 中已存在的名称。 +- `reason` 取值为 `default`（自动触发开启）、`skill`（该 skill 被单独指定）或 + `all`（被常驻策略覆盖）。`all` 优先于 `skill`。 +- `publications` 仅出现在 `off` 与 `on` 中。`status` 取值为 `published`、 + `skipped`（目标不由 `oo` 管理）或 `failed`。 + ### `oo skills recommend` 为内置 `oo` skill 提供收尾阶段的 skill 推荐，并提供静音控制。内置 `oo` skill 会 diff --git a/src/application/commands/skills/auto-sync.ts b/src/application/commands/skills/auto-sync.ts index 3f64f7d..ab3797a 100644 --- a/src/application/commands/skills/auto-sync.ts +++ b/src/application/commands/skills/auto-sync.ts @@ -1,4 +1,5 @@ import type { CliExecutionContext } from "../../contracts/cli.ts"; +import type { SkillAutoTriggerPolicy } from "./auto-trigger-policy.ts"; import type { BundledSkillName } from "./embedded-assets.ts"; import type { ManagedSkillHost, @@ -7,6 +8,7 @@ import type { import type { RegistrySkillMetadata } from "./skill-metadata.ts"; import { join } from "node:path"; +import { readSkillAutoTriggerPolicy } from "./auto-trigger-policy.ts"; import { publishBundledSkillInstallation, } from "./bundled-skill-filesystem.ts"; @@ -55,7 +57,11 @@ export async function synchronizeManagedSkillsForAvailableHosts( } await Promise.all([ - synchronizeBundledSkills(hosts, context), + synchronizeBundledSkills( + hosts, + context, + await readSkillAutoTriggerPolicy(context.settingsStore), + ), synchronizeRegistrySkills(hosts, context), ]); } @@ -72,11 +78,12 @@ export async function synchronizeManagedSkillsForAvailableHosts( async function synchronizeBundledSkills( hosts: readonly ManagedSkillHost[], context: SkillSyncContext, + autoTriggerPolicy: SkillAutoTriggerPolicy, ): Promise { await Promise.all( hosts.flatMap(host => availableBundledSkillNames.map(skillName => - synchronizeBundledSkill(host, skillName, context), + synchronizeBundledSkill(host, skillName, context, autoTriggerPolicy), ), ), ); @@ -86,6 +93,7 @@ async function synchronizeBundledSkill( host: ManagedSkillHost, skillName: BundledSkillName, context: SkillSyncContext, + autoTriggerPolicy: SkillAutoTriggerPolicy, ): Promise { const settingsFilePath = context.settingsStore.getFilePath(); const installation = resolveManagedSkillHostInstallation(host, skillName); @@ -162,6 +170,7 @@ async function synchronizeBundledSkill( const installedSkillDirectoryPath = await publishManagedBundledSkill({ agentName: host.agentName, + autoTriggerPolicy, homeDirectory: host.homeDirectory, settingsFilePath, skillName, diff --git a/src/application/commands/skills/auto-trigger-policy.test.ts b/src/application/commands/skills/auto-trigger-policy.test.ts new file mode 100644 index 0000000..ef32335 --- /dev/null +++ b/src/application/commands/skills/auto-trigger-policy.test.ts @@ -0,0 +1,145 @@ +import { describe, expect, test } from "bun:test"; + +import { + createSkillAutoTriggerRenderVariables, + defaultSkillAutoTriggerPolicy, + isSkillAutoTriggerEnabled, + readSkillAutoTriggerReason, + resolveSkillAutoTriggerPolicy, +} from "./auto-trigger-policy.ts"; +import { + availableBundledSkillAgentNames, + availableBundledSkillNames, + getBundledSkillFiles, + readBundledSkillFileContent, +} from "./embedded-assets.ts"; + +describe("skill auto-trigger policy", () => { + test("leaves every bundled skill auto-triggering by default", () => { + for (const skillName of availableBundledSkillNames) { + expect( + isSkillAutoTriggerEnabled(defaultSkillAutoTriggerPolicy, skillName), + ).toBeTrue(); + expect( + readSkillAutoTriggerReason(defaultSkillAutoTriggerPolicy, skillName), + ).toBe("default"); + } + }); + + test("reads the standing policy as covering every skill, named or not", () => { + const policy = { disabled: [], disabledAll: true }; + + for (const skillName of availableBundledSkillNames) { + expect(isSkillAutoTriggerEnabled(policy, skillName)).toBeFalse(); + expect(readSkillAutoTriggerReason(policy, skillName)).toBe("all"); + } + }); + + test("reads a named skill as disabled and leaves the rest alone", () => { + const policy = { disabled: ["oo-create-skill"], disabledAll: false }; + + expect(isSkillAutoTriggerEnabled(policy, "oo-create-skill")).toBeFalse(); + expect(readSkillAutoTriggerReason(policy, "oo-create-skill")).toBe("skill"); + expect(isSkillAutoTriggerEnabled(policy, "oo")).toBeTrue(); + expect(readSkillAutoTriggerReason(policy, "oo")).toBe("default"); + }); + + test("lets the standing policy outrank a per-skill entry", () => { + const policy = { disabled: ["oo-create-skill"], disabledAll: true }; + + expect(readSkillAutoTriggerReason(policy, "oo")).toBe("all"); + expect(readSkillAutoTriggerReason(policy, "oo-create-skill")).toBe("all"); + }); + + test("resolves the policy from settings", () => { + expect(resolveSkillAutoTriggerPolicy({})).toEqual(defaultSkillAutoTriggerPolicy); + expect( + resolveSkillAutoTriggerPolicy({ + skills: { auto_trigger: { disabled: ["oo"], disabled_all: true } }, + }), + ).toEqual({ disabled: ["oo"], disabledAll: true }); + }); + + test("keeps the two vendor markers inverse to each other", () => { + expect(createSkillAutoTriggerRenderVariables(true)).toEqual({ + allowImplicitInvocation: "true", + disableModelInvocation: "false", + }); + expect(createSkillAutoTriggerRenderVariables(false)).toEqual({ + allowImplicitInvocation: "false", + disableModelInvocation: "true", + }); + }); +}); + +describe("bundled skill auto-trigger markers", () => { + // Every bundled skill needs both markers wired to the render variables. A + // template that hardcodes a value, or omits the frontmatter field, would + // silently ignore the policy for that one skill, so all four are asserted. + test("renders both markers for every bundled skill and agent when auto-trigger is on", async () => { + for (const skillName of availableBundledSkillNames) { + for (const agentName of availableBundledSkillAgentNames) { + expect( + await readMarkerContent(skillName, agentName, "SKILL.md"), + ).toContain("disable-model-invocation: false"); + expect( + await readMarkerContent(skillName, agentName, "agents/openai.yaml"), + ).toContain("allow_implicit_invocation: true"); + } + } + }); + + test("flips both markers for every bundled skill and agent when the standing policy is off", async () => { + const policy = { disabled: [], disabledAll: true }; + + for (const skillName of availableBundledSkillNames) { + for (const agentName of availableBundledSkillAgentNames) { + expect( + await readMarkerContent(skillName, agentName, "SKILL.md", policy), + ).toContain("disable-model-invocation: true"); + expect( + await readMarkerContent( + skillName, + agentName, + "agents/openai.yaml", + policy, + ), + ).toContain("allow_implicit_invocation: false"); + } + } + }); + + test("flips the markers only for the named skill", async () => { + const policy = { disabled: ["oo-create-skill"], disabledAll: false }; + + expect( + await readMarkerContent("oo-create-skill", "claude", "SKILL.md", policy), + ).toContain("disable-model-invocation: true"); + expect( + await readMarkerContent("oo-create-skill", "claude", "agents/openai.yaml", policy), + ).toContain("allow_implicit_invocation: false"); + expect( + await readMarkerContent("oo", "claude", "SKILL.md", policy), + ).toContain("disable-model-invocation: false"); + expect( + await readMarkerContent("oo", "claude", "agents/openai.yaml", policy), + ).toContain("allow_implicit_invocation: true"); + }); +}); + +async function readMarkerContent( + skillName: (typeof availableBundledSkillNames)[number], + agentName: (typeof availableBundledSkillAgentNames)[number], + relativePath: string, + policy?: { disabled: readonly string[]; disabledAll: boolean }, +): Promise { + const file = getBundledSkillFiles(skillName, agentName).find( + candidate => candidate.relativePath === relativePath, + ); + + if (file === undefined) { + throw new Error(`Missing bundled skill file ${relativePath} for ${skillName}.`); + } + + return await readBundledSkillFileContent(file, policy); +} diff --git a/src/application/commands/skills/auto-trigger-policy.ts b/src/application/commands/skills/auto-trigger-policy.ts new file mode 100644 index 0000000..206d801 --- /dev/null +++ b/src/application/commands/skills/auto-trigger-policy.ts @@ -0,0 +1,81 @@ +import type { SettingsStore } from "../../contracts/settings-store.ts"; +import type { AppSettings } from "../../schemas/settings.ts"; + +import { + getAutoTriggerDisabledSkills, + isSkillAutoTriggerDisabledForAll, +} from "../../schemas/settings.ts"; + +/** + * Whether an agent may load a bundled skill without the user naming it. + * + * This is a materialization input, not a runtime switch: the effective value is + * baked into the published skill files, so it only reaches an agent when the + * bundled skill is republished. `disabledAll` is a standing policy that covers + * bundled skills added by later releases, so it is kept as its own flag rather + * than being expanded into the names known today. + */ +export interface SkillAutoTriggerPolicy { + readonly disabled: readonly string[]; + readonly disabledAll: boolean; +} + +/** Why one skill ended up with the auto-trigger state it has. */ +export type SkillAutoTriggerReason = "all" | "default" | "skill"; + +export const defaultSkillAutoTriggerPolicy: SkillAutoTriggerPolicy = { + disabled: [], + disabledAll: false, +}; + +export function resolveSkillAutoTriggerPolicy( + settings: AppSettings, +): SkillAutoTriggerPolicy { + return { + disabled: getAutoTriggerDisabledSkills(settings), + disabledAll: isSkillAutoTriggerDisabledForAll(settings), + }; +} + +/** + * Reads the policy once for a whole invocation. Publication loops fan out over + * hosts and skills, and the settings store is uncached, so resolving inside the + * loop would re-read and re-parse the file per target and let one invocation + * straddle two different policies. + */ +export async function readSkillAutoTriggerPolicy( + settingsStore: Pick, +): Promise { + return resolveSkillAutoTriggerPolicy(await settingsStore.read()); +} + +export function isSkillAutoTriggerEnabled( + policy: SkillAutoTriggerPolicy, + skillName: string, +): boolean { + return readSkillAutoTriggerReason(policy, skillName) === "default"; +} + +export function readSkillAutoTriggerReason( + policy: SkillAutoTriggerPolicy, + skillName: string, +): SkillAutoTriggerReason { + if (policy.disabledAll) { + return "all"; + } + + return policy.disabled.includes(skillName) ? "skill" : "default"; +} + +// Claude Code reads `disable-model-invocation` from the SKILL.md frontmatter; +// Codex reads `policy.allow_implicit_invocation` from `agents/openai.yaml`. The +// two vendors spell the same policy with opposite polarity, so both values are +// derived here from one boolean instead of being written independently. +export function createSkillAutoTriggerRenderVariables( + autoTriggerEnabled: boolean, +): Record { + return { + allowImplicitInvocation: String(autoTriggerEnabled), + disableModelInvocation: String(!autoTriggerEnabled), + }; +} diff --git a/src/application/commands/skills/auto-trigger/index.cli.test.ts b/src/application/commands/skills/auto-trigger/index.cli.test.ts new file mode 100644 index 0000000..a3fcb91 --- /dev/null +++ b/src/application/commands/skills/auto-trigger/index.cli.test.ts @@ -0,0 +1,673 @@ +import type { CliSandbox } from "../../../../../__tests__/helpers.ts"; +import type { BundledSkillAgentName, BundledSkillName } from "../embedded-assets.ts"; + +import { mkdir } from "node:fs/promises"; +import { dirname, join } from "node:path"; +import { describe, expect, test } from "bun:test"; + +import { createCliSandbox } from "../../../../../__tests__/helpers.ts"; +import { resolveStorePaths } from "../../../../adapters/store/store-path.ts"; +import { APP_NAME } from "../../../config/app-config.ts"; +import { + parseTelemetryRowPayload, + readTelemetryRowsForTest, +} from "../../../telemetry/outbox.ts"; +import { + resolveBundledSkillCanonicalDirectoryPath, + resolveBundledSkillCanonicalRootDirectoryPath, +} from "../bundled-skill-paths.ts"; +import { resolveManagedSkillAgentHomeDirectory } from "../managed-skill-agents.ts"; + +describe("skills auto-trigger CLI", () => { + test("--json reports every bundled skill as auto-triggering by default", async () => { + const sandbox = await createCliSandbox(); + + try { + const result = await sandbox.run( + ["skills", "auto-trigger", "status", "--json"], + ); + + expect(result.exitCode).toBe(0); + expect(JSON.parse(result.stdout)).toEqual({ + disabled: [], + disabledAll: false, + skills: [ + { autoTrigger: true, name: "oo", reason: "default" }, + { autoTrigger: true, name: "oo-find-skills", reason: "default" }, + { autoTrigger: true, name: "oo-create-skill", reason: "default" }, + { autoTrigger: true, name: "oo-publish-skill", reason: "default" }, + ], + }); + } + finally { + await sandbox.cleanup(); + } + }); + + test("off --all flips both vendor markers in every host", async () => { + const sandbox = await createCliSandbox(); + + try { + await createClaudeHome(sandbox); + + const result = await sandbox.run( + ["skills", "auto-trigger", "off", "--all", "--json"], + ); + const payload = JSON.parse(result.stdout); + + expect(result.exitCode).toBe(0); + expect(payload.disabledAll).toBeTrue(); + expect(payload.disabled).toEqual([]); + expect(payload.skills.every((skill: { autoTrigger: boolean }) => + !skill.autoTrigger, + )).toBeTrue(); + expect(payload.publications).toHaveLength(8); + expect(payload.publications.every((publication: { status: string }) => + publication.status === "published", + )).toBeTrue(); + + for (const agentName of ["universal", "claude"] as const) { + expect( + await readSkillMarkdown(sandbox, agentName, "oo"), + ).toContain("disable-model-invocation: true"); + expect( + await readOpenAiPolicy(sandbox, agentName, "oo"), + ).toContain("allow_implicit_invocation: false"); + } + } + finally { + await sandbox.cleanup(); + } + }); + + test("off with a skill name leaves the other bundled skills alone", async () => { + const sandbox = await createCliSandbox(); + + try { + const result = await sandbox.run( + ["skills", "auto-trigger", "off", "oo-create-skill", "--json"], + ); + const payload = JSON.parse(result.stdout); + + expect(result.exitCode).toBe(0); + expect(payload.disabledAll).toBeFalse(); + expect(payload.disabled).toEqual(["oo-create-skill"]); + expect( + await readSkillMarkdown(sandbox, "universal", "oo-create-skill"), + ).toContain("disable-model-invocation: true"); + expect( + await readSkillMarkdown(sandbox, "universal", "oo"), + ).toContain("disable-model-invocation: false"); + } + finally { + await sandbox.cleanup(); + } + }); + + test("on --all clears the standing policy and the per-skill list together", async () => { + const sandbox = await createCliSandbox(); + + try { + await sandbox.run(["skills", "auto-trigger", "off", "oo-create-skill"]); + await sandbox.run(["skills", "auto-trigger", "off", "--all"]); + + const result = await sandbox.run( + ["skills", "auto-trigger", "on", "--all", "--json"], + ); + const payload = JSON.parse(result.stdout); + + expect(result.exitCode).toBe(0); + expect(payload.disabledAll).toBeFalse(); + expect(payload.disabled).toEqual([]); + expect( + await readSkillMarkdown(sandbox, "universal", "oo-create-skill"), + ).toContain("disable-model-invocation: false"); + } + finally { + await sandbox.cleanup(); + } + }); + + test("on with a skill name removes only that entry", async () => { + const sandbox = await createCliSandbox(); + + try { + await sandbox.run([ + "skills", + "auto-trigger", + "off", + "oo-create-skill", + "oo-publish-skill", + ]); + + const result = await sandbox.run( + ["skills", "auto-trigger", "on", "oo-create-skill", "--json"], + ); + + expect(result.exitCode).toBe(0); + expect(JSON.parse(result.stdout).disabled).toEqual(["oo-publish-skill"]); + } + finally { + await sandbox.cleanup(); + } + }); + + test("repeating off is idempotent", async () => { + const sandbox = await createCliSandbox(); + + try { + await sandbox.run(["skills", "auto-trigger", "off", "oo-create-skill"]); + const result = await sandbox.run( + ["skills", "auto-trigger", "off", "oo-create-skill", "--json"], + ); + + expect(result.exitCode).toBe(0); + expect(JSON.parse(result.stdout).disabled).toEqual(["oo-create-skill"]); + } + finally { + await sandbox.cleanup(); + } + }); + + test("rejects passing both skill names and --all", async () => { + const sandbox = await createCliSandbox(); + + try { + const result = await sandbox.run( + ["skills", "auto-trigger", "off", "oo", "--all"], + ); + + expect(result.exitCode).toBe(2); + expect(result.stderr).toContain("not both"); + } + finally { + await sandbox.cleanup(); + } + }); + + test("rejects passing neither skill names nor --all", async () => { + const sandbox = await createCliSandbox(); + + try { + const result = await sandbox.run(["skills", "auto-trigger", "on"]); + + expect(result.exitCode).toBe(2); + expect(result.stderr).toContain("at least one"); + } + finally { + await sandbox.cleanup(); + } + }); + + test("rejects a skill name that is not bundled", async () => { + const sandbox = await createCliSandbox(); + + try { + const result = await sandbox.run( + ["skills", "auto-trigger", "off", "oo-gmail"], + ); + + expect(result.exitCode).toBe(2); + expect(result.stderr).toContain("Unknown bundled skill: oo-gmail."); + expect(result.stderr).toContain("oo-publish-skill"); + } + finally { + await sandbox.cleanup(); + } + }); + + // A bundled skill dropped by a later release would otherwise be stuck in the + // settings file: `off` will not name it, and `on --all` is too blunt. + test("lets on remove a stored name that is no longer a bundled skill", async () => { + const sandbox = await createCliSandbox(); + + try { + await sandbox.run(["skills", "auto-trigger", "status"]); + + const settingsFilePath = resolveStorePaths({ + appName: APP_NAME, + env: sandbox.env, + platform: process.platform, + }).settingsFilePath; + + await Bun.write( + settingsFilePath, + `${await Bun.file(settingsFilePath).text()}\n[skills.auto_trigger]\ndisabled = ["oo-retired-skill", "oo"]\n`, + ); + + const result = await sandbox.run( + ["skills", "auto-trigger", "on", "oo-retired-skill", "--json"], + ); + + expect(result.exitCode).toBe(0); + expect(JSON.parse(result.stdout).disabled).toEqual(["oo"]); + } + finally { + await sandbox.cleanup(); + } + }); + + test("off still rejects a name that is not a bundled skill", async () => { + const sandbox = await createCliSandbox(); + + try { + await sandbox.run(["skills", "auto-trigger", "status"]); + + const settingsFilePath = resolveStorePaths({ + appName: APP_NAME, + env: sandbox.env, + platform: process.platform, + }).settingsFilePath; + + await Bun.write( + settingsFilePath, + `${await Bun.file(settingsFilePath).text()}\n[skills.auto_trigger]\ndisabled = ["oo-retired-skill"]\n`, + ); + + const result = await sandbox.run( + ["skills", "auto-trigger", "off", "oo-retired-skill"], + ); + + expect(result.exitCode).toBe(2); + expect(result.stderr).toContain("Unknown bundled skill: oo-retired-skill."); + } + finally { + await sandbox.cleanup(); + } + }); + + test("skips a same-name host directory that oo does not manage", async () => { + const sandbox = await createCliSandbox(); + + try { + const claudeHomeDirectory = await createClaudeHome(sandbox); + + await mkdir(join(claudeHomeDirectory, "skills", "oo"), { recursive: true }); + await Bun.write( + join(claudeHomeDirectory, "skills", "oo", "SKILL.md"), + "---\nname: oo\ndescription: hand written\n---\n", + ); + + const result = await sandbox.run( + ["skills", "auto-trigger", "off", "--all", "--json"], + ); + const payload = JSON.parse(result.stdout); + + expect(result.exitCode).toBe(0); + expect(payload.publications).toContainEqual({ + agent: "claude", + skill: "oo", + status: "skipped", + }); + expect( + await readSkillMarkdown(sandbox, "claude", "oo"), + ).toBe("---\nname: oo\ndescription: hand written\n---\n"); + expect( + await readSkillMarkdown(sandbox, "universal", "oo"), + ).toContain("disable-model-invocation: true"); + } + finally { + await sandbox.cleanup(); + } + }); + + test("names the skipped targets in text output", async () => { + const sandbox = await createCliSandbox(); + + try { + const claudeHomeDirectory = await createClaudeHome(sandbox); + + await mkdir(join(claudeHomeDirectory, "skills", "oo"), { recursive: true }); + await Bun.write( + join(claudeHomeDirectory, "skills", "oo", "SKILL.md"), + "---\nname: oo\n---\n", + ); + + const result = await sandbox.run(["skills", "auto-trigger", "off", "--all"]); + + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain("Every bundled skill is now manual-only."); + expect(result.stdout).toContain("skipped 1 target(s) not managed by oo"); + expect(result.stdout).toContain("Claude Code/oo"); + } + finally { + await sandbox.cleanup(); + } + }); + + test("keeps the canonical copy in step with the host copy", async () => { + const sandbox = await createCliSandbox(); + + try { + await sandbox.run(["skills", "auto-trigger", "off", "--all"]); + + const canonicalDirectoryPath = resolveBundledSkillCanonicalDirectoryPath( + resolveStorePaths({ + appName: APP_NAME, + env: sandbox.env, + platform: process.platform, + }).settingsFilePath, + "oo", + "universal", + ); + + expect( + await Bun.file(join(canonicalDirectoryPath, "SKILL.md")).text(), + ).toBe(await readSkillMarkdown(sandbox, "universal", "oo")); + expect( + await Bun.file( + join(canonicalDirectoryPath, "agents", "openai.yaml"), + ).text(), + ).toContain("allow_implicit_invocation: false"); + } + finally { + await sandbox.cleanup(); + } + }); + + // `oo skills install` resolves the policy through its own read rather than + // through the auto-trigger command, so a skill installed into a host that + // appeared after the policy was set must still come out manual-only. + test("installs into a newly detected host with the stored policy applied", async () => { + const sandbox = await createCliSandbox(); + + try { + await sandbox.run(["skills", "auto-trigger", "off", "--all"]); + await createClaudeHome(sandbox); + + const result = await sandbox.run(["skills", "install"]); + + expect(result.exitCode).toBe(0); + expect( + await readSkillMarkdown(sandbox, "claude", "oo"), + ).toContain("disable-model-invocation: true"); + expect( + await readOpenAiPolicy(sandbox, "claude", "oo"), + ).toContain("allow_implicit_invocation: false"); + } + finally { + await sandbox.cleanup(); + } + }); + + // The documented escape hatch: startup synchronization deliberately ignores + // a hand-edited policy, so `oo skills repair` has to be the thing that + // applies it. If that stops working the setting silently never takes. + test("applies a hand-edited policy through oo skills repair", async () => { + const sandbox = await createCliSandbox(); + + try { + await sandbox.run(["skills", "auto-trigger", "status"]); + + const settingsFilePath = resolveStorePaths({ + appName: APP_NAME, + env: sandbox.env, + platform: process.platform, + }).settingsFilePath; + + await Bun.write( + settingsFilePath, + `${await Bun.file(settingsFilePath).text()}\n[skills.auto_trigger]\ndisabled = ["oo-create-skill"]\n`, + ); + + expect( + await readSkillMarkdown(sandbox, "universal", "oo-create-skill"), + ).toContain("disable-model-invocation: false"); + + const result = await sandbox.run( + ["skills", "repair", "--skill", "oo-create-skill"], + ); + + expect(result.exitCode).toBe(0); + expect( + await readSkillMarkdown(sandbox, "universal", "oo-create-skill"), + ).toContain("disable-model-invocation: true"); + expect( + await readOpenAiPolicy(sandbox, "universal", "oo-create-skill"), + ).toContain("allow_implicit_invocation: false"); + } + finally { + await sandbox.cleanup(); + } + }); + + // Without this note the success line reads as though the skill went back to + // auto-triggering, while `disabled_all` is still suppressing every skill. + test("says the standing policy still applies after a per-skill on", async () => { + const sandbox = await createCliSandbox(); + + try { + await sandbox.run(["skills", "auto-trigger", "off", "--all"]); + const result = await sandbox.run(["skills", "auto-trigger", "on", "oo"]); + + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain( + "every bundled skill stays manual-only", + ); + expect( + await readSkillMarkdown(sandbox, "universal", "oo"), + ).toContain("disable-model-invocation: true"); + } + finally { + await sandbox.cleanup(); + } + }); + + test("omits the standing policy note when no standing policy is set", async () => { + const sandbox = await createCliSandbox(); + + try { + const result = await sandbox.run( + ["skills", "auto-trigger", "off", "oo-create-skill"], + ); + + expect(result.exitCode).toBe(0); + expect(result.stdout).not.toContain("stays manual-only"); + } + finally { + await sandbox.cleanup(); + } + }); + + // The manifest in telemetry-decisions.test.ts only checks the declared + // names against the forbidden lists; nothing there observes the real call, + // so the recorded shape is asserted here. + test("records only bucketed auto-trigger dimensions, never skill names", async () => { + const sandbox = await createCliSandbox(); + + try { + const storePaths = resolveStorePaths({ + appName: APP_NAME, + env: sandbox.env, + platform: process.platform, + }); + + await sandbox.run(["skills", "auto-trigger", "off", "oo-create-skill"]); + + const properties = readCommandTelemetryProperties( + storePaths.telemetryDirectory, + ); + + expect(properties).toMatchObject({ + command_full: "skills.auto-trigger.off", + skill_count_bucket: "1-5", + target_scope: "skills", + }); + + for (const forbidden of [ + "disabled", + "skill_ids_sample", + "skill_name", + "skill_names", + "skills", + ]) { + expect(properties).not.toHaveProperty(forbidden); + } + + expect(JSON.stringify(properties)).not.toContain("oo-create-skill"); + } + finally { + await sandbox.cleanup(); + } + }); + + test("records the standing policy flag and a bucketed count from status", async () => { + const sandbox = await createCliSandbox(); + + try { + const storePaths = resolveStorePaths({ + appName: APP_NAME, + env: sandbox.env, + platform: process.platform, + }); + + await sandbox.run(["skills", "auto-trigger", "off", "--all"]); + await sandbox.run(["skills", "auto-trigger", "status"]); + + const properties = readCommandTelemetryProperties( + storePaths.telemetryDirectory, + ); + + expect(properties).toMatchObject({ + command_full: "skills.auto-trigger.status", + disabled_all: true, + disabled_count_bucket: "0", + }); + } + finally { + await sandbox.cleanup(); + } + }); + + // The half-applied state is the whole reason this command exits non-zero. + // Pins four things at once: the JSON payload is emitted BEFORE the throw so + // a script still learns which targets were left auto-triggering, the exit + // code is 1 rather than 2, the recovery hint names the failed skills (a + // bare `oo skills repair` exits 2), and the saved setting survives. + test("reports the half-applied state and exits 1 when publication fails", async () => { + const sandbox = await createCliSandbox(); + + try { + const canonicalRootDirectoryPath + = resolveBundledSkillCanonicalRootDirectoryPath( + resolveStorePaths({ + appName: APP_NAME, + env: sandbox.env, + platform: process.platform, + }).settingsFilePath, + "universal", + ); + + // A regular file where the canonical root directory belongs makes + // every universal publication fail without needing permission bits, + // which do not behave the same way across platforms and CI users. + await mkdir(dirname(canonicalRootDirectoryPath), { recursive: true }); + await Bun.write(canonicalRootDirectoryPath, "not a directory"); + + const result = await sandbox.run( + ["skills", "auto-trigger", "off", "--all", "--json"], + ); + const payload = JSON.parse(result.stdout); + + expect(result.exitCode).toBe(1); + expect(payload.disabledAll).toBeTrue(); + expect( + payload.publications.filter( + (publication: { status: string }) => publication.status === "failed", + ), + ).toHaveLength(4); + expect(result.stderr).toContain("could not be republished"); + expect(result.stderr).toContain("oo skills repair --skill oo"); + + const status = await sandbox.run( + ["skills", "auto-trigger", "status", "--json"], + ); + + expect(JSON.parse(status.stdout).disabledAll).toBeTrue(); + } + finally { + await sandbox.cleanup(); + } + }); + + test("status reports the reason each skill ended up manual-only", async () => { + const sandbox = await createCliSandbox(); + + try { + await sandbox.run(["skills", "auto-trigger", "off", "oo-create-skill"]); + const result = await sandbox.run(["skills", "auto-trigger", "status"]); + + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain("Auto-trigger is off for 1 bundled skill(s)."); + expect(result.stdout).toContain("oo: auto"); + expect(result.stdout).toContain("oo-create-skill: manual"); + } + finally { + await sandbox.cleanup(); + } + }); +}); + +// The command event is the last row written by the invocation under test. +function readCommandTelemetryProperties( + telemetryDirectory: string, +): Record { + const row = readTelemetryRowsForTest(telemetryDirectory).at(-1); + + if (row === undefined) { + throw new Error("No telemetry row was recorded."); + } + + const payload = parseTelemetryRowPayload(row); + + if (payload === undefined) { + throw new Error("The telemetry row payload could not be parsed."); + } + + return payload.properties as Record; +} + +async function createClaudeHome(sandbox: CliSandbox): Promise { + const homeDirectory = resolveManagedSkillAgentHomeDirectory(sandbox.env, "claude"); + + await mkdir(homeDirectory, { recursive: true }); + + return homeDirectory; +} + +async function readSkillMarkdown( + sandbox: CliSandbox, + agentName: BundledSkillAgentName, + skillName: BundledSkillName, +): Promise { + return await readHostSkillFile(sandbox, agentName, skillName, "SKILL.md"); +} + +async function readOpenAiPolicy( + sandbox: CliSandbox, + agentName: BundledSkillAgentName, + skillName: BundledSkillName, +): Promise { + return await readHostSkillFile( + sandbox, + agentName, + skillName, + join("agents", "openai.yaml"), + ); +} + +async function readHostSkillFile( + sandbox: CliSandbox, + agentName: BundledSkillAgentName, + skillName: BundledSkillName, + relativePath: string, +): Promise { + return await Bun.file( + join( + resolveManagedSkillAgentHomeDirectory(sandbox.env, agentName), + "skills", + skillName, + relativePath, + ), + ).text(); +} diff --git a/src/application/commands/skills/auto-trigger/index.ts b/src/application/commands/skills/auto-trigger/index.ts new file mode 100644 index 0000000..bfc3b2d --- /dev/null +++ b/src/application/commands/skills/auto-trigger/index.ts @@ -0,0 +1,16 @@ +import type { CliCommandDefinition } from "../../../contracts/cli.ts"; + +import { skillsAutoTriggerOffCommand } from "./off.ts"; +import { skillsAutoTriggerOnCommand } from "./on.ts"; +import { skillsAutoTriggerStatusCommand } from "./status.ts"; + +export const skillsAutoTriggerCommand: CliCommandDefinition = { + name: "auto-trigger", + summaryKey: "commands.skills.autoTrigger.summary", + descriptionKey: "commands.skills.autoTrigger.description", + children: [ + skillsAutoTriggerOffCommand, + skillsAutoTriggerOnCommand, + skillsAutoTriggerStatusCommand, + ], +}; diff --git a/src/application/commands/skills/auto-trigger/off.ts b/src/application/commands/skills/auto-trigger/off.ts new file mode 100644 index 0000000..656e321 --- /dev/null +++ b/src/application/commands/skills/auto-trigger/off.ts @@ -0,0 +1,13 @@ +import { + addAutoTriggerDisabledSkills, + setSkillAutoTriggerDisabledForAll, +} from "../../../schemas/settings.ts"; +import { createAutoTriggerToggleCommand } from "./toggle-command.ts"; + +export const skillsAutoTriggerOffCommand = createAutoTriggerToggleCommand({ + acceptsPersistedName: false, + applyAll: settings => setSkillAutoTriggerDisabledForAll(settings, true), + applySkills: (settings, skillNames) => + addAutoTriggerDisabledSkills(settings, skillNames), + name: "off", +}); diff --git a/src/application/commands/skills/auto-trigger/on.ts b/src/application/commands/skills/auto-trigger/on.ts new file mode 100644 index 0000000..f3ce2db --- /dev/null +++ b/src/application/commands/skills/auto-trigger/on.ts @@ -0,0 +1,15 @@ +import { + removeAutoTriggerDisabledSkills, + setSkillAutoTriggerDisabledForAll, +} from "../../../schemas/settings.ts"; +import { createAutoTriggerToggleCommand } from "./toggle-command.ts"; + +export const skillsAutoTriggerOnCommand = createAutoTriggerToggleCommand({ + acceptsPersistedName: true, + // `on --all` restores the shipped default outright: it clears the standing + // policy and the per-skill list, so nothing is left silently manual-only. + applyAll: settings => setSkillAutoTriggerDisabledForAll(settings, false), + applySkills: (settings, skillNames) => + removeAutoTriggerDisabledSkills(settings, skillNames), + name: "on", +}); diff --git a/src/application/commands/skills/auto-trigger/publish.ts b/src/application/commands/skills/auto-trigger/publish.ts new file mode 100644 index 0000000..df4be67 --- /dev/null +++ b/src/application/commands/skills/auto-trigger/publish.ts @@ -0,0 +1,115 @@ +import type { CliExecutionContext } from "../../../contracts/cli.ts"; +import type { SkillAutoTriggerPolicy } from "../auto-trigger-policy.ts"; +import type { BundledSkillName } from "../embedded-assets.ts"; +import type { ManagedSkillHost } from "../managed-skill-hosts.ts"; +import type { SkillAutoTriggerPublication } from "./report.ts"; + +import { resolveBundledSkillCanonicalDirectoryPath } from "../bundled-skill-paths.ts"; +import { availableBundledSkillNames } from "../embedded-assets.ts"; +import { + createMissingManagedSkillHostError, + resolveAvailableManagedSkillHosts, + resolveManagedSkillHostInstallation, +} from "../managed-skill-hosts.ts"; +import { + isUnmanagedBundledSkillDirectory, + publishManagedBundledSkill, +} from "../shared.ts"; + +type SkillAutoTriggerPublishContext = Pick< + CliExecutionContext, + "env" | "logger" | "settingsStore" | "version" +>; + +/** + * Rewrites every bundled skill for every available host so the policy that was + * just persisted reaches disk. + * + * A same-name directory that oo does not manage is skipped rather than + * overwritten, the way startup synchronization treats it. This deliberately + * differs from `oo skills install`, which aborts the whole run on such a + * directory: the user is changing a preference, not installing, so one + * hand-made directory must not stop the policy reaching the other agents. The + * caller reports the skipped targets. Anything else that goes wrong is recorded + * as a failure so the caller can exit non-zero — a half-applied policy that + * reported success would leave the user believing skills are manual-only when + * some agent still auto-triggers them. + */ +export async function publishBundledSkillsForAutoTrigger( + context: SkillAutoTriggerPublishContext, + autoTriggerPolicy: SkillAutoTriggerPolicy, +): Promise { + const hosts = await resolveAvailableManagedSkillHosts(context.env); + + if (hosts.length === 0) { + throw createMissingManagedSkillHostError(context.env); + } + + const publications = await Promise.all( + hosts.flatMap(host => + availableBundledSkillNames.map(skillName => + publishOneBundledSkill(host, skillName, autoTriggerPolicy, context), + ), + ), + ); + + return publications; +} + +async function publishOneBundledSkill( + host: ManagedSkillHost, + skillName: BundledSkillName, + autoTriggerPolicy: SkillAutoTriggerPolicy, + context: SkillAutoTriggerPublishContext, +): Promise { + const settingsFilePath = context.settingsStore.getFilePath(); + const installation = resolveManagedSkillHostInstallation(host, skillName); + const logFields = { + agentName: host.agentName, + path: installation.installedSkillDirectoryPath, + skillName, + }; + + try { + const unmanaged = await Promise.all([ + isUnmanagedBundledSkillDirectory( + installation.installedSkillDirectoryPath, + ), + isUnmanagedBundledSkillDirectory( + resolveBundledSkillCanonicalDirectoryPath( + settingsFilePath, + skillName, + host.agentName, + ), + ), + ]); + + if (unmanaged.includes(true)) { + context.logger.warn( + logFields, + "Auto-trigger publication skipped because the target is not managed by oo.", + ); + + return { agent: host.agentName, skill: skillName, status: "skipped" }; + } + + await publishManagedBundledSkill({ + agentName: host.agentName, + autoTriggerPolicy, + homeDirectory: host.homeDirectory, + settingsFilePath, + skillName, + version: context.version, + }); + + return { agent: host.agentName, skill: skillName, status: "published" }; + } + catch (error) { + context.logger.warn( + { ...logFields, err: error }, + "Auto-trigger publication failed.", + ); + + return { agent: host.agentName, skill: skillName, status: "failed" }; + } +} diff --git a/src/application/commands/skills/auto-trigger/report.ts b/src/application/commands/skills/auto-trigger/report.ts new file mode 100644 index 0000000..ae1ac2a --- /dev/null +++ b/src/application/commands/skills/auto-trigger/report.ts @@ -0,0 +1,50 @@ +import type { + SkillAutoTriggerPolicy, + SkillAutoTriggerReason, +} from "../auto-trigger-policy.ts"; +import type { BundledSkillAgentName, BundledSkillName } from "../embedded-assets.ts"; + +import { readSkillAutoTriggerReason } from "../auto-trigger-policy.ts"; +import { availableBundledSkillNames } from "../embedded-assets.ts"; + +type SkillAutoTriggerPublicationStatus = "failed" | "published" | "skipped"; + +export interface SkillAutoTriggerPublication { + agent: BundledSkillAgentName; + skill: BundledSkillName; + status: SkillAutoTriggerPublicationStatus; +} + +interface SkillAutoTriggerSkillState { + autoTrigger: boolean; + name: BundledSkillName; + reason: SkillAutoTriggerReason; +} + +export interface SkillAutoTriggerState { + disabled: readonly string[]; + disabledAll: boolean; + skills: readonly SkillAutoTriggerSkillState[]; +} + +// Projects the stored policy onto the bundled skills that exist in this +// release. `disabled` is echoed verbatim rather than filtered: a name left over +// from a release that shipped a different bundled skill is still what the +// settings file says, and hiding it would make the output disagree with disk. +export function readSkillAutoTriggerState( + policy: SkillAutoTriggerPolicy, +): SkillAutoTriggerState { + return { + disabled: policy.disabled, + disabledAll: policy.disabledAll, + skills: availableBundledSkillNames.map((name) => { + const reason = readSkillAutoTriggerReason(policy, name); + + return { + autoTrigger: reason === "default", + name, + reason, + } satisfies SkillAutoTriggerSkillState; + }), + }; +} diff --git a/src/application/commands/skills/auto-trigger/status.ts b/src/application/commands/skills/auto-trigger/status.ts new file mode 100644 index 0000000..1d9fd56 --- /dev/null +++ b/src/application/commands/skills/auto-trigger/status.ts @@ -0,0 +1,73 @@ +import type { CliCommandDefinition, CliExecutionContext } from "../../../contracts/cli.ts"; +import type { SkillAutoTriggerState } from "./report.ts"; + +import { z } from "zod"; +import { bucketTelemetryCount } from "../../../telemetry/buckets.ts"; +import { writeLine } from "../../shared/output.ts"; +import { resolveSkillAutoTriggerPolicy } from "../auto-trigger-policy.ts"; +import { readSkillAutoTriggerState } from "./report.ts"; + +export const skillsAutoTriggerStatusCommand: CliCommandDefinition< + Record +> = { + name: "status", + summaryKey: "commands.skills.autoTrigger.status.summary", + descriptionKey: "commands.skills.autoTrigger.status.description", + output: "standard", + inputSchema: z.object({}), + handler: async (_input, context) => { + const state = readSkillAutoTriggerState( + resolveSkillAutoTriggerPolicy(await context.settingsStore.read()), + ); + + context.telemetry?.recordProperties({ + disabled_all: state.disabledAll, + disabled_count_bucket: bucketTelemetryCount(state.disabled.length), + }); + + context.output.emit(state, () => { + writeStatusText(context, state); + }); + }, +}; + +function writeStatusText( + context: Pick, + state: SkillAutoTriggerState, +): void { + const disabledCount = state.skills.filter(skill => !skill.autoTrigger).length; + + writeLine( + context.stdout, + context.translator.t(readStatusHeaderKey(state, disabledCount), { + count: disabledCount, + }), + ); + + for (const skill of state.skills) { + writeLine( + context.stdout, + context.translator.t("skills.autoTrigger.status.line", { + name: skill.name, + state: context.translator.t( + `skills.autoTrigger.state.${skill.reason}`, + ), + }), + ); + } +} + +function readStatusHeaderKey( + state: SkillAutoTriggerState, + disabledCount: number, +): string { + if (state.disabledAll) { + return "skills.autoTrigger.status.headerAll"; + } + + if (disabledCount > 0) { + return "skills.autoTrigger.status.headerSkills"; + } + + return "skills.autoTrigger.status.headerDefault"; +} diff --git a/src/application/commands/skills/auto-trigger/toggle-command.ts b/src/application/commands/skills/auto-trigger/toggle-command.ts new file mode 100644 index 0000000..31ed53a --- /dev/null +++ b/src/application/commands/skills/auto-trigger/toggle-command.ts @@ -0,0 +1,226 @@ +import type { CliCommandDefinition, CliExecutionContext } from "../../../contracts/cli.ts"; +import type { AppSettings } from "../../../schemas/settings.ts"; +import type { SkillAutoTriggerPublication } from "./report.ts"; + +import { z } from "zod"; +import { CliUserError } from "../../../contracts/cli.ts"; +import { getAutoTriggerDisabledSkills } from "../../../schemas/settings.ts"; +import { bucketTelemetryCount } from "../../../telemetry/buckets.ts"; +import { writeLine } from "../../shared/output.ts"; +import { resolveSkillAutoTriggerPolicy } from "../auto-trigger-policy.ts"; +import { availableBundledSkillNames } from "../embedded-assets.ts"; +import { readManagedSkillAgentLabel } from "../managed-skill-agents.ts"; +import { isBundledSkillName } from "../shared.ts"; +import { publishBundledSkillsForAutoTrigger } from "./publish.ts"; +import { readSkillAutoTriggerState } from "./report.ts"; + +interface AutoTriggerToggleInput { + all?: boolean; + skillNames?: string[]; +} + +interface AutoTriggerToggleConfig { + /** + * Whether the command may be handed a name that is not a bundled skill in + * this release. `off` may not — you cannot make a skill that does not exist + * manual-only, and accepting the name would hide a typo. `on` may, but only + * for a name the stored list actually holds: a bundled skill dropped by a + * later release would otherwise be stuck in the settings file with no way + * to remove it except `on --all`, which discards every other choice. + */ + acceptsPersistedName: boolean; + applyAll: (settings: AppSettings) => AppSettings; + applySkills: ( + settings: AppSettings, + skillNames: readonly string[], + ) => AppSettings; + name: "off" | "on"; +} + +// Builds the `off`/`on` pair. Both parse, validate, persist, republish, and +// report identically; only the settings mutation differs, so it is injected. +export function createAutoTriggerToggleCommand( + config: AutoTriggerToggleConfig, +): CliCommandDefinition { + return { + name: config.name, + summaryKey: `commands.skills.autoTrigger.${config.name}.summary`, + descriptionKey: `commands.skills.autoTrigger.${config.name}.description`, + arguments: [ + { + name: "skillNames", + descriptionKey: `arguments.skills.autoTrigger.${config.name}.skillName`, + required: false, + variadic: true, + }, + ], + options: [ + { + name: "all", + longFlag: "--all", + descriptionKey: `options.skills.autoTrigger.${config.name}.all`, + }, + ], + output: "standard", + inputSchema: z.object({ + all: z.boolean().optional(), + skillNames: z.array(z.string()).optional(), + }), + handler: async (input, context) => { + const all = input.all === true; + const skillNames = [...new Set(input.skillNames ?? [])]; + + if (all && skillNames.length > 0) { + throw new CliUserError("errors.skills.autoTrigger.conflictingScope", 2); + } + + if (!all && skillNames.length === 0) { + throw new CliUserError("errors.skills.autoTrigger.missingScope", 2); + } + + const persisted = getAutoTriggerDisabledSkills( + await context.settingsStore.read(), + ); + + for (const skillName of skillNames) { + const accepted = isBundledSkillName(skillName) + || (config.acceptsPersistedName && persisted.includes(skillName)); + + if (!accepted) { + throw new CliUserError("errors.skills.autoTrigger.unknownSkill", 2, { + skills: availableBundledSkillNames.join(", "), + value: skillName, + }); + } + } + + context.telemetry?.recordProperties({ + skill_count_bucket: bucketTelemetryCount(skillNames.length), + target_scope: all ? "all" : "skills", + }); + + // Settings first: they are the single record of what the user asked + // for, so a publication that fails half-way still leaves a state + // `oo skills repair` can finish applying. + const next = await context.settingsStore.update(settings => + all + ? config.applyAll(settings) + : config.applySkills(settings, skillNames), + ); + const policy = resolveSkillAutoTriggerPolicy(next); + const publications = await publishBundledSkillsForAutoTrigger( + context, + policy, + ); + + const state = readSkillAutoTriggerState(policy); + + context.output.emit({ ...state, publications }, () => { + writeToggleText(context, { + all, + name: config.name, + publications, + skillNames, + standingPolicy: state.disabledAll, + }); + }); + + const failed = publications.filter( + publication => publication.status === "failed", + ); + + if (failed.length > 0) { + throw new CliUserError("errors.skills.autoTrigger.publishFailed", 1, { + count: failed.length, + repairArguments: formatRepairArguments(failed), + targets: formatPublicationTargets(failed, context), + }); + } + }, + }; +} + +function writeToggleText( + context: Pick, + options: { + all: boolean; + name: AutoTriggerToggleConfig["name"]; + publications: readonly SkillAutoTriggerPublication[]; + skillNames: readonly string[]; + standingPolicy: boolean; + }, +): void { + const { all, publications } = options; + + writeLine( + context.stdout, + context.translator.t( + `skills.autoTrigger.${options.name}.success.${all ? "all" : "skills"}`, + { + count: options.skillNames.length, + skills: options.skillNames.join(", "), + }, + ), + ); + + // A per-skill change while `--all` is in force does not alter what any agent + // sees. Saying only "these skills can auto-trigger again" would be a plain + // lie, so the standing policy is spelled out with the way to lift it. + if (!all && options.standingPolicy) { + writeLine( + context.stdout, + context.translator.t("skills.autoTrigger.standingPolicyNote"), + ); + } + + const published = publications.filter( + publication => publication.status === "published", + ); + + if (published.length > 0) { + writeLine( + context.stdout, + context.translator.t("skills.autoTrigger.publishedLine", { + agents: new Set(published.map(publication => publication.agent)).size, + count: published.length, + }), + ); + } + + const skipped = publications.filter( + publication => publication.status === "skipped", + ); + + if (skipped.length > 0) { + writeLine( + context.stdout, + context.translator.t("skills.autoTrigger.skippedLine", { + count: skipped.length, + targets: formatPublicationTargets(skipped, context), + }), + ); + } +} + +// `oo skills repair` requires at least one `--skill`, so the recovery hint has +// to name the skills that actually failed rather than suggest a bare command +// the user cannot run. +function formatRepairArguments( + publications: readonly SkillAutoTriggerPublication[], +): string { + return Array.from(new Set(publications.map(publication => publication.skill)), skillName => `--skill ${skillName}`) + .join(" "); +} + +// `/` — never a filesystem path, which text output for +// skills deliberately keeps out of stdout. +function formatPublicationTargets( + publications: readonly SkillAutoTriggerPublication[], + context: Pick, +): string { + return publications + .map(publication => + `${readManagedSkillAgentLabel(publication.agent, context.translator)}/${publication.skill}`, + ) + .join(", "); +} diff --git a/src/application/commands/skills/embedded-assets.test.ts b/src/application/commands/skills/embedded-assets.test.ts index 5932ea9..2226cb5 100644 --- a/src/application/commands/skills/embedded-assets.test.ts +++ b/src/application/commands/skills/embedded-assets.test.ts @@ -22,6 +22,7 @@ describe("embedded skill assets", () => { ]); expect(getBundledSkillFiles("oo", "universal").map(file => file.relativePath)).toEqual([ "SKILL.md", + "agents/openai.yaml", "references/auth-and-billing.md", "references/llm-client.md", "references/search-and-selection.md", @@ -30,6 +31,7 @@ describe("embedded skill assets", () => { ]); expect(getBundledSkillFiles("oo", "claude").map(file => file.relativePath)).toEqual([ "SKILL.md", + "agents/openai.yaml", "references/auth-and-billing.md", "references/llm-client.md", "references/search-and-selection.md", @@ -38,6 +40,7 @@ describe("embedded skill assets", () => { ]); expect(getBundledSkillFiles("oo", "hermes").map(file => file.relativePath)).toEqual([ "SKILL.md", + "agents/openai.yaml", "references/auth-and-billing.md", "references/llm-client.md", "references/search-and-selection.md", @@ -46,6 +49,7 @@ describe("embedded skill assets", () => { ]); expect(getBundledSkillFiles("oo", "codebuddy").map(file => file.relativePath)).toEqual([ "SKILL.md", + "agents/openai.yaml", "references/auth-and-billing.md", "references/llm-client.md", "references/search-and-selection.md", @@ -54,6 +58,7 @@ describe("embedded skill assets", () => { ]); expect(getBundledSkillFiles("oo", "workbuddy").map(file => file.relativePath)).toEqual([ "SKILL.md", + "agents/openai.yaml", "references/auth-and-billing.md", "references/llm-client.md", "references/search-and-selection.md", @@ -62,6 +67,7 @@ describe("embedded skill assets", () => { ]); expect(getBundledSkillFiles("oo", "trae").map(file => file.relativePath)).toEqual([ "SKILL.md", + "agents/openai.yaml", "references/auth-and-billing.md", "references/llm-client.md", "references/search-and-selection.md", @@ -70,6 +76,7 @@ describe("embedded skill assets", () => { ]); expect(getBundledSkillFiles("oo", "trae-cn").map(file => file.relativePath)).toEqual([ "SKILL.md", + "agents/openai.yaml", "references/auth-and-billing.md", "references/llm-client.md", "references/search-and-selection.md", @@ -78,6 +85,7 @@ describe("embedded skill assets", () => { ]); expect(getBundledSkillFiles("oo", "openclaw").map(file => file.relativePath)).toEqual([ "SKILL.md", + "agents/openai.yaml", "references/auth-and-billing.md", "references/llm-client.md", "references/search-and-selection.md", @@ -86,6 +94,7 @@ describe("embedded skill assets", () => { ]); expect(getBundledSkillFiles("oo", "qoderwork").map(file => file.relativePath)).toEqual([ "SKILL.md", + "agents/openai.yaml", "references/auth-and-billing.md", "references/llm-client.md", "references/search-and-selection.md", @@ -94,6 +103,7 @@ describe("embedded skill assets", () => { ]); expect(getBundledSkillFiles("oo", "deepseek-tui").map(file => file.relativePath)).toEqual([ "SKILL.md", + "agents/openai.yaml", "references/auth-and-billing.md", "references/llm-client.md", "references/search-and-selection.md", @@ -106,6 +116,7 @@ describe("embedded skill assets", () => { ), ).toEqual([ "SKILL.md", + "agents/openai.yaml", "references/oo-cli-contract.md", ]); expect( @@ -114,6 +125,7 @@ describe("embedded skill assets", () => { ), ).toEqual([ "SKILL.md", + "agents/openai.yaml", "references/oo-cli-contract.md", ]); expect( @@ -122,6 +134,7 @@ describe("embedded skill assets", () => { ), ).toEqual([ "SKILL.md", + "agents/openai.yaml", "references/oo-cli-contract.md", ]); expect( @@ -130,6 +143,7 @@ describe("embedded skill assets", () => { ), ).toEqual([ "SKILL.md", + "agents/openai.yaml", "references/oo-cli-contract.md", ]); expect( @@ -138,6 +152,7 @@ describe("embedded skill assets", () => { ), ).toEqual([ "SKILL.md", + "agents/openai.yaml", "references/oo-cli-contract.md", ]); expect( @@ -146,6 +161,7 @@ describe("embedded skill assets", () => { ), ).toEqual([ "SKILL.md", + "agents/openai.yaml", "references/oo-cli-contract.md", ]); expect( @@ -154,6 +170,7 @@ describe("embedded skill assets", () => { ), ).toEqual([ "SKILL.md", + "agents/openai.yaml", "references/oo-cli-contract.md", ]); expect( @@ -162,6 +179,7 @@ describe("embedded skill assets", () => { ), ).toEqual([ "SKILL.md", + "agents/openai.yaml", "references/oo-cli-contract.md", ]); expect( @@ -170,6 +188,7 @@ describe("embedded skill assets", () => { ), ).toEqual([ "SKILL.md", + "agents/openai.yaml", "references/oo-cli-contract.md", ]); expect( @@ -178,6 +197,7 @@ describe("embedded skill assets", () => { ), ).toEqual([ "SKILL.md", + "agents/openai.yaml", "references/oo-cli-contract.md", ]); for (const agentName of availableBundledSkillAgentNames) { @@ -187,6 +207,7 @@ describe("embedded skill assets", () => { ), ).toEqual([ "SKILL.md", + posix.join("agents", "openai.yaml"), posix.join("references", "skill-authoring.md"), posix.join("references", "existing-workflow.md"), posix.join("references", "oo-powered.md"), @@ -198,6 +219,7 @@ describe("embedded skill assets", () => { ), ).toEqual([ "SKILL.md", + "agents/openai.yaml", ]); expect( getBundledSkillFiles("oo-publish-skill", "claude").map( @@ -205,6 +227,7 @@ describe("embedded skill assets", () => { ), ).toEqual([ "SKILL.md", + "agents/openai.yaml", ]); expect( getBundledSkillFiles("oo-publish-skill", "hermes").map( @@ -212,6 +235,7 @@ describe("embedded skill assets", () => { ), ).toEqual([ "SKILL.md", + "agents/openai.yaml", ]); expect( getBundledSkillFiles("oo-publish-skill", "codebuddy").map( @@ -219,6 +243,7 @@ describe("embedded skill assets", () => { ), ).toEqual([ "SKILL.md", + "agents/openai.yaml", ]); expect( getBundledSkillFiles("oo-publish-skill", "workbuddy").map( @@ -226,6 +251,7 @@ describe("embedded skill assets", () => { ), ).toEqual([ "SKILL.md", + "agents/openai.yaml", ]); expect( getBundledSkillFiles("oo-publish-skill", "trae").map( @@ -233,6 +259,7 @@ describe("embedded skill assets", () => { ), ).toEqual([ "SKILL.md", + "agents/openai.yaml", ]); expect( getBundledSkillFiles("oo-publish-skill", "trae-cn").map( @@ -240,6 +267,7 @@ describe("embedded skill assets", () => { ), ).toEqual([ "SKILL.md", + "agents/openai.yaml", ]); expect( getBundledSkillFiles("oo-publish-skill", "openclaw").map( @@ -247,6 +275,7 @@ describe("embedded skill assets", () => { ), ).toEqual([ "SKILL.md", + "agents/openai.yaml", ]); expect( getBundledSkillFiles("oo-publish-skill", "qoderwork").map( @@ -254,6 +283,7 @@ describe("embedded skill assets", () => { ), ).toEqual([ "SKILL.md", + "agents/openai.yaml", ]); expect( getBundledSkillFiles("oo-publish-skill", "deepseek-tui").map( @@ -261,6 +291,7 @@ describe("embedded skill assets", () => { ), ).toEqual([ "SKILL.md", + "agents/openai.yaml", ]); }); diff --git a/src/application/commands/skills/embedded-assets.ts b/src/application/commands/skills/embedded-assets.ts index a72945a..f4f944a 100644 --- a/src/application/commands/skills/embedded-assets.ts +++ b/src/application/commands/skills/embedded-assets.ts @@ -1,22 +1,32 @@ +import type { SkillAutoTriggerPolicy } from "./auto-trigger-policy.ts"; import type { BundledSkillAgentName } from "./managed-skill-agents.ts"; import { mkdir } from "node:fs/promises"; import { dirname, join, posix } from "node:path"; import { render } from "agentic-markdown"; +import ooCreateSkillOpenAiPolicyPath from "../../../../contrib/skills/shared/oo-create-skill/agents/openai.yaml" with { type: "file" }; import ooCreateSkillExistingWorkflowPath from "../../../../contrib/skills/shared/oo-create-skill/references/existing-workflow.md" with { type: "file" }; import ooCreateSkillOoPoweredPath from "../../../../contrib/skills/shared/oo-create-skill/references/oo-powered.md" with { type: "file" }; import ooCreateSkillAuthoringPath from "../../../../contrib/skills/shared/oo-create-skill/references/skill-authoring.md" with { type: "file" }; import ooCreateSkillPath from "../../../../contrib/skills/shared/oo-create-skill/SKILL.md" with { type: "file" }; +import ooFindSkillsOpenAiPolicyPath from "../../../../contrib/skills/shared/oo-find-skills/agents/openai.yaml" with { type: "file" }; import ooFindSkillsCliContractPath from "../../../../contrib/skills/shared/oo-find-skills/references/oo-cli-contract.md" with { type: "file" }; import ooFindSkillsSkillPath from "../../../../contrib/skills/shared/oo-find-skills/SKILL.md" with { type: "file" }; +import ooPublishSkillOpenAiPolicyPath from "../../../../contrib/skills/shared/oo-publish-skill/agents/openai.yaml" with { type: "file" }; import ooPublishSkillPath from "../../../../contrib/skills/shared/oo-publish-skill/SKILL.md" with { type: "file" }; +import ooOpenAiPolicyPath from "../../../../contrib/skills/shared/oo/agents/openai.yaml" with { type: "file" }; import ooAuthAndBillingReferencePath from "../../../../contrib/skills/shared/oo/references/auth-and-billing.md" with { type: "file" }; import ooConnectorExecutionReferencePath from "../../../../contrib/skills/shared/oo/references/connector-execution.md" with { type: "file" }; import ooFileTransferReferencePath from "../../../../contrib/skills/shared/oo/references/file-transfer.md" with { type: "file" }; import ooLlmClientReferencePath from "../../../../contrib/skills/shared/oo/references/llm-client.md" with { type: "file" }; import ooSearchAndSelectionReferencePath from "../../../../contrib/skills/shared/oo/references/search-and-selection.md" with { type: "file" }; import ooSkillPath from "../../../../contrib/skills/shared/oo/SKILL.md" with { type: "file" }; +import { + createSkillAutoTriggerRenderVariables, + defaultSkillAutoTriggerPolicy, + isSkillAutoTriggerEnabled, +} from "./auto-trigger-policy.ts"; import { removePath } from "./bundled-skill-filesystem.ts"; import { @@ -49,9 +59,16 @@ export interface BundledSkillFile extends BundledSkillSourceFile { readonly skillName: BundledSkillName; } +// `agents/openai.yaml` is part of every bundled skill regardless of the +// auto-trigger policy: the file always states the policy explicitly, so the +// file list stays static and switching the policy never has to add or remove a +// file from an already-published skill directory. +const openAiPolicyRelativePath = posix.join("agents", "openai.yaml"); + const bundledSkillRegistry = { "oo": createAgentDefinitions([ - createAgenticMarkdownFile("SKILL.md", ooSkillPath), + createRenderedBundledSkillFile("SKILL.md", ooSkillPath), + createRenderedBundledSkillFile(openAiPolicyRelativePath, ooOpenAiPolicyPath), ...createOoReferenceFiles({ authAndBilling: ooAuthAndBillingReferencePath, connectorExecution: ooConnectorExecutionReferencePath, @@ -61,17 +78,20 @@ const bundledSkillRegistry = { }), ]), "oo-create-skill": createAgentDefinitions([ - createAgenticMarkdownFile("SKILL.md", ooCreateSkillPath), - createAgenticMarkdownFile(posix.join("references", "skill-authoring.md"), ooCreateSkillAuthoringPath), - createAgenticMarkdownFile(posix.join("references", "existing-workflow.md"), ooCreateSkillExistingWorkflowPath), - createAgenticMarkdownFile(posix.join("references", "oo-powered.md"), ooCreateSkillOoPoweredPath), + createRenderedBundledSkillFile("SKILL.md", ooCreateSkillPath), + createRenderedBundledSkillFile(openAiPolicyRelativePath, ooCreateSkillOpenAiPolicyPath), + createRenderedBundledSkillFile(posix.join("references", "skill-authoring.md"), ooCreateSkillAuthoringPath), + createRenderedBundledSkillFile(posix.join("references", "existing-workflow.md"), ooCreateSkillExistingWorkflowPath), + createRenderedBundledSkillFile(posix.join("references", "oo-powered.md"), ooCreateSkillOoPoweredPath), ]), "oo-find-skills": createAgentDefinitions([ - createAgenticMarkdownFile("SKILL.md", ooFindSkillsSkillPath), - createAgenticMarkdownFile("references/oo-cli-contract.md", ooFindSkillsCliContractPath), + createRenderedBundledSkillFile("SKILL.md", ooFindSkillsSkillPath), + createRenderedBundledSkillFile(openAiPolicyRelativePath, ooFindSkillsOpenAiPolicyPath), + createRenderedBundledSkillFile("references/oo-cli-contract.md", ooFindSkillsCliContractPath), ]), "oo-publish-skill": createAgentDefinitions([ - createAgenticMarkdownFile("SKILL.md", ooPublishSkillPath), + createRenderedBundledSkillFile("SKILL.md", ooPublishSkillPath), + createRenderedBundledSkillFile(openAiPolicyRelativePath, ooPublishSkillOpenAiPolicyPath), ]), } as const satisfies Record< BundledSkillName, @@ -98,6 +118,12 @@ export function getBundledSkillFiles( // marker. The per-skill directory is removed and recreated so stale files from // a previous export do not linger; sibling content in the parent directory is // left untouched. Returns the written file relative paths in registry order. +// +// The export deliberately renders at the shipped auto-trigger default rather +// than this machine's policy. The output is a portable artifact — vendored into +// a repository, inspected, handed to someone else — and none of the commands +// that manage the policy can reach it afterwards, so baking one installation's +// preference into it would travel silently to everyone who consumed it. export async function materializeBundledSkillToDirectory(options: { agentName: BundledSkillAgentName; skillName: BundledSkillName; @@ -121,14 +147,23 @@ export async function materializeBundledSkillToDirectory(options: { return files.map(file => file.relativePath); } +// Renders one bundled skill file for its agent. `agentic-markdown` throws on a +// referenced variable it was not given, so every variable used by any bundled +// template has to be supplied here. The auto-trigger policy defaults to the +// shipped default rather than being required: an omitted policy means "nothing +// has been configured", which is exactly what the default expresses. export async function readBundledSkillFileContent( file: BundledSkillFile, + autoTriggerPolicy: SkillAutoTriggerPolicy = defaultSkillAutoTriggerPolicy, ): Promise { const content = await Bun.file(file.sourcePath).text(); const agent = readManagedSkillAgent(file.agentName); const variables: Record = { agent: file.agentName, agentTitle: agent.title, + ...createSkillAutoTriggerRenderVariables( + isSkillAutoTriggerEnabled(autoTriggerPolicy, file.skillName), + ), }; if (agent.skillSelectionPromptTool !== undefined) { @@ -157,15 +192,15 @@ function createOoReferenceFiles(sourcePaths: { searchAndSelection: string; }): readonly BundledSkillSourceFile[] { return [ - createAgenticMarkdownFile("references/auth-and-billing.md", sourcePaths.authAndBilling), - createAgenticMarkdownFile("references/llm-client.md", sourcePaths.llmClient), - createAgenticMarkdownFile("references/search-and-selection.md", sourcePaths.searchAndSelection), - createAgenticMarkdownFile("references/connector-execution.md", sourcePaths.connectorExecution), - createAgenticMarkdownFile("references/file-transfer.md", sourcePaths.fileTransfer), + createRenderedBundledSkillFile("references/auth-and-billing.md", sourcePaths.authAndBilling), + createRenderedBundledSkillFile("references/llm-client.md", sourcePaths.llmClient), + createRenderedBundledSkillFile("references/search-and-selection.md", sourcePaths.searchAndSelection), + createRenderedBundledSkillFile("references/connector-execution.md", sourcePaths.connectorExecution), + createRenderedBundledSkillFile("references/file-transfer.md", sourcePaths.fileTransfer), ]; } -function createAgenticMarkdownFile( +function createRenderedBundledSkillFile( relativePath: string, sourcePath: string, ): BundledSkillSourceFile { diff --git a/src/application/commands/skills/index.test.ts b/src/application/commands/skills/index.test.ts index ec4e7ea..a6fea4f 100644 --- a/src/application/commands/skills/index.test.ts +++ b/src/application/commands/skills/index.test.ts @@ -667,11 +667,12 @@ describe("skills commands", () => { expect(await readFile(metadataFilePath, "utf8")).toBe( renderSkillMetadataJson(createBundledSkillMetadata("9.9.9")), ); - await expect( - stat(join(skillDirectoryPath, "agents", "openai.yaml")), - ).rejects.toMatchObject({ - code: "ENOENT", - }); + expect( + await readFile( + join(skillDirectoryPath, "agents", "openai.yaml"), + "utf8", + ), + ).toContain("allow_implicit_invocation: true"); } finally { await sandbox.cleanup(); @@ -726,11 +727,12 @@ describe("skills commands", () => { expect(installedSkillMarkdown).toBe( await readBundledSkillFileContent(hermesSkillFile), ); - await expect( - stat(join(skillDirectoryPath, "agents", "openai.yaml")), - ).rejects.toMatchObject({ - code: "ENOENT", - }); + expect( + await readFile( + join(skillDirectoryPath, "agents", "openai.yaml"), + "utf8", + ), + ).toContain("allow_implicit_invocation: true"); } finally { await sandbox.cleanup(); @@ -773,11 +775,12 @@ describe("skills commands", () => { expect(await readFile(metadataFilePath, "utf8")).toBe( renderSkillMetadataJson(createBundledSkillMetadata("9.9.9")), ); - await expect( - stat(join(skillDirectoryPath, "agents", "openai.yaml")), - ).rejects.toMatchObject({ - code: "ENOENT", - }); + expect( + await readFile( + join(skillDirectoryPath, "agents", "openai.yaml"), + "utf8", + ), + ).toContain("allow_implicit_invocation: true"); } finally { await sandbox.cleanup(); @@ -838,11 +841,12 @@ describe("skills commands", () => { await readBundledSkillFileContent(qoderWorkSkillFile), ); expect(installedSkillMarkdown).not.toContain("allowed-tools"); - await expect( - stat(join(skillDirectoryPath, "agents", "openai.yaml")), - ).rejects.toMatchObject({ - code: "ENOENT", - }); + expect( + await readFile( + join(skillDirectoryPath, "agents", "openai.yaml"), + "utf8", + ), + ).toContain("allow_implicit_invocation: true"); } finally { await sandbox.cleanup(); @@ -895,11 +899,12 @@ describe("skills commands", () => { expect(await readFile(skillFilePath, "utf8")).toBe( await readBundledSkillFileContent(codeBuddySkillFile), ); - await expect( - stat(join(skillDirectoryPath, "agents", "openai.yaml")), - ).rejects.toMatchObject({ - code: "ENOENT", - }); + expect( + await readFile( + join(skillDirectoryPath, "agents", "openai.yaml"), + "utf8", + ), + ).toContain("allow_implicit_invocation: true"); } finally { await sandbox.cleanup(); @@ -957,11 +962,12 @@ describe("skills commands", () => { expect(await readFile(skillFilePath, "utf8")).toBe( await readBundledSkillFileContent(deepSeekTuiSkillFile), ); - await expect( - stat(join(skillDirectoryPath, "agents", "openai.yaml")), - ).rejects.toMatchObject({ - code: "ENOENT", - }); + expect( + await readFile( + join(skillDirectoryPath, "agents", "openai.yaml"), + "utf8", + ), + ).toContain("allow_implicit_invocation: true"); } finally { await sandbox.cleanup(); @@ -1014,11 +1020,12 @@ describe("skills commands", () => { expect(await readFile(skillFilePath, "utf8")).toBe( await readBundledSkillFileContent(workBuddySkillFile), ); - await expect( - stat(join(skillDirectoryPath, "agents", "openai.yaml")), - ).rejects.toMatchObject({ - code: "ENOENT", - }); + expect( + await readFile( + join(skillDirectoryPath, "agents", "openai.yaml"), + "utf8", + ), + ).toContain("allow_implicit_invocation: true"); } finally { await sandbox.cleanup(); @@ -1076,11 +1083,12 @@ describe("skills commands", () => { expect(await readFile(skillFilePath, "utf8")).toBe( await readBundledSkillFileContent(traeSkillFile), ); - await expect( - stat(join(skillDirectoryPath, "agents", "openai.yaml")), - ).rejects.toMatchObject({ - code: "ENOENT", - }); + expect( + await readFile( + join(skillDirectoryPath, "agents", "openai.yaml"), + "utf8", + ), + ).toContain("allow_implicit_invocation: true"); } finally { await sandbox.cleanup(); @@ -1138,11 +1146,12 @@ describe("skills commands", () => { expect(await readFile(skillFilePath, "utf8")).toBe( await readBundledSkillFileContent(traeCnSkillFile), ); - await expect( - stat(join(skillDirectoryPath, "agents", "openai.yaml")), - ).rejects.toMatchObject({ - code: "ENOENT", - }); + expect( + await readFile( + join(skillDirectoryPath, "agents", "openai.yaml"), + "utf8", + ), + ).toContain("allow_implicit_invocation: true"); } finally { await sandbox.cleanup(); diff --git a/src/application/commands/skills/index.ts b/src/application/commands/skills/index.ts index 854a8be..5c20d76 100644 --- a/src/application/commands/skills/index.ts +++ b/src/application/commands/skills/index.ts @@ -1,6 +1,7 @@ import type { CliCommandDefinition } from "../../contracts/cli.ts"; import { skillsAdoptCommand } from "./adopt.ts"; +import { skillsAutoTriggerCommand } from "./auto-trigger/index.ts"; import { skillsCheckUpdateCommand } from "./check-update.ts"; import { skillsCheckCommand } from "./check.ts"; import { skillsInitCommand } from "./init.ts"; @@ -37,6 +38,7 @@ export const skillsCommand: CliCommandDefinition = { skillsUninstallCommand, skillsRepairCommand, skillsCheckUpdateCommand, + skillsAutoTriggerCommand, skillsRecommendCommand, ], }; diff --git a/src/application/commands/skills/install.cli.test.ts b/src/application/commands/skills/install.cli.test.ts index b3d0047..c28709a 100644 --- a/src/application/commands/skills/install.cli.test.ts +++ b/src/application/commands/skills/install.cli.test.ts @@ -497,6 +497,33 @@ describe("skills install --out-dir export", () => { "oo-publish-skill", ]; + // The export is a portable artifact that none of the auto-trigger commands + // can reach afterwards, so it must not carry this installation's policy to + // whoever consumes the directory. + test("exports at the shipped auto-trigger default even when it is turned off locally", async () => { + const sandbox = await createCliSandbox(); + const outDir = await mkdtemp(join(tmpdir(), "oo-out-")); + + try { + await sandbox.run(["skills", "auto-trigger", "off", "--all"]); + sandbox.env.OO_SKILLS_SYNC_DISABLED = "1"; + + const result = await sandbox.run(["skills", "add", "--out-dir", outDir]); + + expect(result.exitCode).toBe(0); + expect(await readFile(join(outDir, "oo", "SKILL.md"), "utf8")).toContain( + "disable-model-invocation: false", + ); + expect( + await readFile(join(outDir, "oo", "agents", "openai.yaml"), "utf8"), + ).toContain("allow_implicit_invocation: true"); + } + finally { + await rm(outDir, { force: true, recursive: true }); + await sandbox.cleanup(); + } + }); + test("exports all bundled skills into the directory without touching agent homes", async () => { const sandbox = await createCliSandbox(); const outDir = await mkdtemp(join(tmpdir(), "oo-out-")); diff --git a/src/application/commands/skills/repair.ts b/src/application/commands/skills/repair.ts index c4cb57c..ccc7274 100644 --- a/src/application/commands/skills/repair.ts +++ b/src/application/commands/skills/repair.ts @@ -2,11 +2,13 @@ import type { CliCommandDefinition, CliExecutionContext, } from "../../contracts/cli.ts"; +import type { SkillAutoTriggerPolicy } from "./auto-trigger-policy.ts"; import type { BundledSkillAgentName, BundledSkillName } from "./embedded-assets.ts"; import { z } from "zod"; import { CliUserError } from "../../contracts/cli.ts"; import { bucketTelemetryCount } from "../../telemetry/buckets.ts"; import { writeLine } from "../shared/output.ts"; +import { readSkillAutoTriggerPolicy } from "./auto-trigger-policy.ts"; import { publishBundledSkillInstallation } from "./bundled-skill-filesystem.ts"; import { directoryExists } from "./bundled-skill-observation.ts"; import { resolveBundledSkillCanonicalDirectoryPath } from "./bundled-skill-paths.ts"; @@ -290,10 +292,15 @@ interface RunRepairOptions { async function runRepair(options: RunRepairOptions): Promise { const pairs: Array> = []; + const autoTriggerPolicy = await readSkillAutoTriggerPolicy( + options.context.settingsStore, + ); for (const source of options.sources) { for (const agentId of options.agents) { - pairs.push(repairPair(source, agentId, options.context)); + pairs.push( + repairPair(source, agentId, options.context, autoTriggerPolicy), + ); } } @@ -325,6 +332,7 @@ async function repairPair( source: RepairSource, agentId: BundledSkillAgentName, context: CliExecutionContext, + autoTriggerPolicy: SkillAutoTriggerPolicy, ): Promise { const homeDirectory = resolveManagedSkillAgentHomeDirectory(context.env, agentId); const settingsFilePath = context.settingsStore.getFilePath(); @@ -369,6 +377,7 @@ async function repairPair( if (source.kind === "bundled") { await publishManagedBundledSkill({ agentName: agentId, + autoTriggerPolicy, homeDirectory, settingsFilePath, skillName: source.skillName, diff --git a/src/application/commands/skills/shared.ts b/src/application/commands/skills/shared.ts index baa2199..e161111 100644 --- a/src/application/commands/skills/shared.ts +++ b/src/application/commands/skills/shared.ts @@ -1,5 +1,6 @@ import type { CliExecutionContext } from "../../contracts/cli.ts"; +import type { SkillAutoTriggerPolicy } from "./auto-trigger-policy.ts"; import type { BundledSkillAgentName, BundledSkillName, @@ -12,6 +13,7 @@ import type { ManagedSkillHostInstallation } from "./managed-skill-hosts.ts"; import { mkdir } from "node:fs/promises"; import { dirname, join } from "node:path"; import { CliUserError } from "../../contracts/cli.ts"; +import { readSkillAutoTriggerPolicy } from "./auto-trigger-policy.ts"; import { publishBundledSkillInstallation, removePath, @@ -64,10 +66,14 @@ export async function installBundledSkill( } const publications: ManagedSkillInstallPublication[] = []; + const autoTriggerPolicy = await readSkillAutoTriggerPolicy( + context.settingsStore, + ); for (const installation of installations) { const installedSkillDirectoryPath = await publishManagedBundledSkill({ agentName: installation.agentName, + autoTriggerPolicy, homeDirectory: installation.homeDirectory, settingsFilePath: context.settingsStore.getFilePath(), skillName, @@ -96,8 +102,13 @@ export async function installBundledSkill( }; } +// Rewrites the canonical copy and republishes it into the agent home. The +// auto-trigger policy is required rather than defaulted: this is the only path +// by which a policy change reaches disk, so a caller that forgets it would +// silently publish skills that ignore the user's configuration. export async function publishManagedBundledSkill(options: { agentName: BundledSkillAgentName; + autoTriggerPolicy: SkillAutoTriggerPolicy; homeDirectory: string; settingsFilePath: string; skillName: BundledSkillName; @@ -141,6 +152,7 @@ export function isScopedPackageName(value: string): boolean { async function writeBundledSkillCanonicalInstallation(options: { agentName: BundledSkillAgentName; + autoTriggerPolicy: SkillAutoTriggerPolicy; homeDirectory: string; settingsFilePath: string; skillName: BundledSkillName; @@ -169,7 +181,10 @@ async function writeBundledSkillCanonicalInstallation(options: { ); await mkdir(dirname(destinationPath), { recursive: true }); - await Bun.write(destinationPath, await readBundledSkillFileContent(file)); + await Bun.write( + destinationPath, + await readBundledSkillFileContent(file, options.autoTriggerPolicy), + ); } await writeInstalledBundledSkillMetadata( @@ -228,7 +243,7 @@ async function validateBundledSkillInstallationTarget( // A directory is present at the path but does not carry bundled oo metadata, // so installing over it would clobber content oo does not own. -async function isUnmanagedBundledSkillDirectory( +export async function isUnmanagedBundledSkillDirectory( skillDirectoryPath: string, ): Promise { const state = await readSkillDirectoryState(skillDirectoryPath); diff --git a/src/application/commands/telemetry-decisions.test.ts b/src/application/commands/telemetry-decisions.test.ts index 817b9ea..93b68b8 100644 --- a/src/application/commands/telemetry-decisions.test.ts +++ b/src/application/commands/telemetry-decisions.test.ts @@ -454,6 +454,34 @@ const commandTelemetryDecisions = { ], reason: "Records bucketed counts, package- and skill-filter usage, and bounded package-name samples; never records skill names, versions, or paths.", }, + "skills.auto-trigger": { + kind: "generic", + reason: "Command group; child commands record safe auto-trigger dimensions.", + }, + "skills.auto-trigger.off": { + kind: "properties", + properties: [ + "target_scope", + "skill_count_bucket", + ], + reason: "Records whether the standing or per-skill scope was used and a bucketed count; the bundled skill names themselves are not recorded.", + }, + "skills.auto-trigger.on": { + kind: "properties", + properties: [ + "target_scope", + "skill_count_bucket", + ], + reason: "Records whether the standing or per-skill scope was used and a bucketed count; the bundled skill names themselves are not recorded.", + }, + "skills.auto-trigger.status": { + kind: "properties", + properties: [ + "disabled_all", + "disabled_count_bucket", + ], + reason: "Records the standing auto-trigger policy flag and a bucketed count of per-skill overrides; never records skill names or paths.", + }, "skills.recommend": { kind: "generic", reason: "Command group; child commands record safe suggestion dimensions.", diff --git a/src/application/schemas/settings.test.ts b/src/application/schemas/settings.test.ts index ed60219..b1ff391 100644 --- a/src/application/schemas/settings.test.ts +++ b/src/application/schemas/settings.test.ts @@ -2,17 +2,139 @@ import { describe, expect, test } from "bun:test"; import { parse as parseToml } from "smol-toml"; import { + addAutoTriggerDisabledSkills, addDismissedSkillRecommendations, + getAutoTriggerDisabledSkills, getDismissedSkillRecommendations, getLegacyIdentityTeam, + isSkillAutoTriggerDisabledForAll, isSkillRecommendationsMuted, + removeAutoTriggerDisabledSkills, removeDismissedSkillRecommendations, renderSettingsFile, + setSkillAutoTriggerDisabledForAll, setSkillRecommendationsMuted, settingsFileReadSchema, unsetLegacyIdentityTeam, } from "./settings.ts"; +describe("skill auto-trigger settings", () => { + test("defaults to auto-trigger enabled with no per-skill entries", () => { + expect(isSkillAutoTriggerDisabledForAll({})).toBe(false); + expect(getAutoTriggerDisabledSkills({})).toEqual([]); + }); + + test("clears the per-skill list when the standing policy is set", () => { + const named = addAutoTriggerDisabledSkills({}, ["oo-create-skill"]); + const all = setSkillAutoTriggerDisabledForAll(named, true); + + expect(isSkillAutoTriggerDisabledForAll(all)).toBe(true); + expect(getAutoTriggerDisabledSkills(all)).toEqual([]); + }); + + test("clears the whole section when the standing policy is lifted", () => { + const all = setSkillAutoTriggerDisabledForAll( + addAutoTriggerDisabledSkills({}, ["oo-create-skill"]), + true, + ); + const cleared = setSkillAutoTriggerDisabledForAll(all, false); + + expect(isSkillAutoTriggerDisabledForAll(cleared)).toBe(false); + expect(getAutoTriggerDisabledSkills(cleared)).toEqual([]); + expect(cleared.skills?.auto_trigger).toBeUndefined(); + }); + + test("sorts and de-duplicates per-skill entries", () => { + const settings = addAutoTriggerDisabledSkills({}, [ + "oo-publish-skill", + "oo-create-skill", + "oo-publish-skill", + ]); + + expect(getAutoTriggerDisabledSkills(settings)).toEqual([ + "oo-create-skill", + "oo-publish-skill", + ]); + }); + + test("prunes the per-skill list once the last entry is removed", () => { + const settings = addAutoTriggerDisabledSkills({}, ["oo-create-skill"]); + const cleared = removeAutoTriggerDisabledSkills(settings, ["oo-create-skill"]); + + expect(getAutoTriggerDisabledSkills(cleared)).toEqual([]); + expect(cleared.skills?.auto_trigger?.disabled).toBeUndefined(); + }); + + test("keeps a standing policy while a per-skill entry is added", () => { + const settings = addAutoTriggerDisabledSkills( + setSkillAutoTriggerDisabledForAll({}, true), + ["oo-create-skill"], + ); + + expect(isSkillAutoTriggerDisabledForAll(settings)).toBe(true); + expect(getAutoTriggerDisabledSkills(settings)).toEqual(["oo-create-skill"]); + }); + + // `[skills]` is shared with `[skills.recommend]`, and lifting the standing + // policy is the one mutation here that deletes a whole subsection. If the + // empty-parent pruning ever over-reaches it would wipe an unrelated mute + // and dismissal list from a command that never mentions recommendations. + test("leaves skills.recommend intact when the standing policy is lifted", () => { + const settings = setSkillAutoTriggerDisabledForAll( + addDismissedSkillRecommendations( + setSkillRecommendationsMuted({}, true), + ["oo-gmail"], + ), + true, + ); + const cleared = setSkillAutoTriggerDisabledForAll(settings, false); + + expect(cleared.skills?.auto_trigger).toBeUndefined(); + expect(isSkillRecommendationsMuted(cleared)).toBe(true); + expect(getDismissedSkillRecommendations(cleared)).toEqual(["oo-gmail"]); + + const rendered = renderSettingsFile(cleared); + + expect(rendered).toContain("\n[skills.recommend]"); + expect(rendered).not.toContain("\n[skills.auto_trigger]"); + }); + + test("renders and round-trips the skills.auto_trigger section", () => { + const settings = addAutoTriggerDisabledSkills( + setSkillAutoTriggerDisabledForAll({}, true), + ["oo-create-skill"], + ); + const rendered = renderSettingsFile(settings); + + expect(rendered).toContain("[skills.auto_trigger]"); + + const parsed = settingsFileReadSchema.parse(parseToml(rendered)); + + expect(parsed.skills?.auto_trigger?.disabled_all).toBe(true); + expect(parsed.skills?.auto_trigger?.disabled).toEqual(["oo-create-skill"]); + }); + + test("does not render an active skills.auto_trigger section by default", () => { + const rendered = renderSettingsFile({}); + + expect(rendered).not.toContain("\n[skills.auto_trigger]"); + expect(rendered).not.toContain("\n[skills]"); + }); + + test("keeps both skills subsections when each holds a value", () => { + const settings = addAutoTriggerDisabledSkills( + setSkillRecommendationsMuted({}, true), + ["oo-create-skill"], + ); + const parsed = settingsFileReadSchema.parse( + parseToml(renderSettingsFile(settings)), + ); + + expect(parsed.skills?.recommend?.muted).toBe(true); + expect(parsed.skills?.auto_trigger?.disabled).toEqual(["oo-create-skill"]); + }); +}); + describe("skill recommendation settings", () => { test("defaults to not muted with no dismissals", () => { expect(isSkillRecommendationsMuted({})).toBe(false); diff --git a/src/application/schemas/settings.ts b/src/application/schemas/settings.ts index 667a956..3bfa17f 100644 --- a/src/application/schemas/settings.ts +++ b/src/application/schemas/settings.ts @@ -51,11 +51,26 @@ const skillsRecommendSettingsShape = { const skillsRecommendSettingsReadSchema = z.object(skillsRecommendSettingsShape); const skillsRecommendSettingsSchema = z.object(skillsRecommendSettingsShape).strict(); +// Standing policy for whether bundled skills may be loaded by an agent without +// the user naming them. `disabled_all` covers every bundled skill, including +// ones added by a later release; `disabled` names individual ones. This is an +// input to skill materialization, not a runtime switch: the effective value is +// baked into the published skill files (see skills/auto-trigger-policy.ts). +const skillsAutoTriggerSettingsShape = { + disabled_all: z.boolean().optional(), + disabled: z.array(z.string()).optional(), +}; + +const skillsAutoTriggerSettingsReadSchema = z.object(skillsAutoTriggerSettingsShape); +const skillsAutoTriggerSettingsSchema = z.object(skillsAutoTriggerSettingsShape).strict(); + const skillsSettingsReadSchema = z.object({ + auto_trigger: skillsAutoTriggerSettingsReadSchema.optional(), recommend: skillsRecommendSettingsReadSchema.optional(), }); const skillsSettingsSchema = z.object({ + auto_trigger: skillsAutoTriggerSettingsSchema.optional(), recommend: skillsRecommendSettingsSchema.optional(), }).strict(); @@ -112,6 +127,17 @@ const defaultSettingsCommentBlocks = [ "# muted = false", "# dismissed = [\"oo-gmail\"]", ], + [ + "# skills.auto_trigger controls whether agents may load a bundled skill without being asked to.", + "# disabled_all: when true, every bundled skill is manual-only, including ones added later. Default: false.", + "# disabled: bundled skill names that are manual-only. Default: none.", + "# Manage these with `oo skills auto-trigger off` and `oo skills auto-trigger on`.", + "# Editing this section by hand does not republish the skill files. To apply a hand-edited value, run:", + "# oo skills repair --skill oo --skill oo-find-skills --skill oo-create-skill --skill oo-publish-skill", + "# [skills.auto_trigger]", + "# disabled_all = false", + "# disabled = [\"oo-create-skill\"]", + ], ] as const; export function renderSettingsFile(settings: AppSettings): string { @@ -148,11 +174,30 @@ export function renderSettingsFile(settings: AppSettings): string { }; } - const recommend = parsedSettings.skills?.recommend; + const persistedSkills = buildPersistedSkillsSection(parsedSettings.skills); + + if (persistedSkills !== undefined) { + persistedSettings.skills = persistedSkills; + } + + const serializedSettings = stringifyToml(persistedSettings).trimEnd(); + + if (serializedSettings !== "") { + lines.push("", serializedSettings); + } + + return `${lines.join("\n")}\n`; +} + +// Both `[skills]` subsections drop keys that hold their implicit default, so an +// installation that never changed them keeps the file at its default shape. A +// subsection with nothing left to say is omitted, and so is `[skills]` itself. +function buildPersistedSkillsSection( + skills: AppSettings["skills"], +): Record | undefined { const persistedRecommend: Record = {}; + const recommend = skills?.recommend; - // Only persist the global mute when it is on; `false` is the implicit - // default and is left out to keep the file at its default shape. if (recommend?.muted === true) { persistedRecommend.muted = true; } @@ -161,17 +206,32 @@ export function renderSettingsFile(settings: AppSettings): string { persistedRecommend.dismissed = recommend.dismissed; } + const persistedAutoTrigger: Record = {}; + const autoTrigger = skills?.auto_trigger; + + if (autoTrigger?.disabled_all === true) { + persistedAutoTrigger.disabled_all = true; + } + + if (autoTrigger?.disabled !== undefined && autoTrigger.disabled.length > 0) { + persistedAutoTrigger.disabled = autoTrigger.disabled; + } + + const persistedSkills: Record = {}; + if (Object.keys(persistedRecommend).length > 0) { - persistedSettings.skills = { recommend: persistedRecommend }; + persistedSkills.recommend = persistedRecommend; } - const serializedSettings = stringifyToml(persistedSettings).trimEnd(); + if (Object.keys(persistedAutoTrigger).length > 0) { + persistedSkills.auto_trigger = persistedAutoTrigger; + } - if (serializedSettings !== "") { - lines.push("", serializedSettings); + if (Object.keys(persistedSkills).length === 0) { + return undefined; } - return `${lines.join("\n")}\n`; + return persistedSkills; } export function collectUnknownSettingsFileKeyPaths( @@ -353,6 +413,91 @@ export function removeDismissedSkillRecommendations( }; } +export function isSkillAutoTriggerDisabledForAll(settings: AppSettings): boolean { + return settings.skills?.auto_trigger?.disabled_all ?? false; +} + +export function getAutoTriggerDisabledSkills( + settings: AppSettings, +): readonly string[] { + return settings.skills?.auto_trigger?.disabled ?? []; +} + +// Writing the standing policy always clears the per-skill list: `--all` in +// either direction is the whole answer, so leaving a list behind would keep a +// second, contradictory record of the same state. +export function setSkillAutoTriggerDisabledForAll( + settings: AppSettings, + disabledAll: boolean, +): AppSettings { + const cleared = deleteNestedProperty(settings, ["skills", "auto_trigger"]); + + if (!disabledAll) { + return cleared; + } + + return { + ...cleared, + skills: { + ...cleared.skills, + auto_trigger: { disabled_all: true }, + }, + }; +} + +// Adds skill names to the per-skill list, keeping the result de-duplicated and +// sorted for a stable settings file. A standing `disabled_all` is left alone; +// it already covers these names, and clearing it here would silently widen the +// command from "these skills" to "all skills". +export function addAutoTriggerDisabledSkills( + settings: AppSettings, + skillNames: readonly string[], +): AppSettings { + const next = sortUnique([ + ...getAutoTriggerDisabledSkills(settings), + ...skillNames, + ]); + + return { + ...settings, + skills: { + ...settings.skills, + auto_trigger: { + ...settings.skills?.auto_trigger, + disabled: next, + }, + }, + }; +} + +export function removeAutoTriggerDisabledSkills( + settings: AppSettings, + skillNames: readonly string[], +): AppSettings { + const removal = new Set(skillNames); + const current = getAutoTriggerDisabledSkills(settings); + const next = current.filter(name => !removal.has(name)); + + if (next.length === current.length) { + return settings; + } + + if (next.length === 0) { + return deleteNestedProperty(settings, ["skills", "auto_trigger", "disabled"]); + } + + return { + ...settings, + skills: { + ...settings.skills, + auto_trigger: { + ...settings.skills?.auto_trigger, + disabled: next, + }, + }, + }; +} + function sortUnique(values: readonly string[]): string[] { return [...new Set(values)].sort((left, right) => left.localeCompare(right)); } diff --git a/src/i18n/catalog.ts b/src/i18n/catalog.ts index d867ab5..526fd76 100644 --- a/src/i18n/catalog.ts +++ b/src/i18n/catalog.ts @@ -852,6 +852,58 @@ export const enMessages = { "skills.checkUpdate.failuresHeader": "Failures:", "skills.checkUpdate.failuresLine": " {skillId}: {message}", + "commands.skills.autoTrigger.description": + "Control whether agents may load a bundled skill on their own. Turning auto-trigger off leaves the skill installed and invocable by name, and republishes the bundled skills so the change reaches every agent.", + "commands.skills.autoTrigger.summary": "Control bundled skill auto-trigger", + "commands.skills.autoTrigger.off.description": + "Make the given bundled skills manual-only, or use --all to make every bundled skill manual-only, including ones added by later releases.", + "commands.skills.autoTrigger.off.summary": "Turn auto-trigger off", + "commands.skills.autoTrigger.on.description": + "Let agents load the given bundled skills on their own again, or use --all to restore the default for every bundled skill.", + "commands.skills.autoTrigger.on.summary": "Turn auto-trigger on", + "commands.skills.autoTrigger.status.description": + "Show the auto-trigger state of every bundled skill.", + "commands.skills.autoTrigger.status.summary": "Show auto-trigger state", + "arguments.skills.autoTrigger.off.skillName": + "Bundled skill name(s) to make manual-only", + "arguments.skills.autoTrigger.on.skillName": + "Bundled skill name(s) to allow auto-trigger for again", + "options.skills.autoTrigger.off.all": + "Make every bundled skill manual-only instead of specific skills", + "options.skills.autoTrigger.on.all": + "Restore the default for every bundled skill instead of specific skills", + "errors.skills.autoTrigger.conflictingScope": + "Pass bundled skill names or --all, not both.", + "errors.skills.autoTrigger.missingScope": + "Pass at least one bundled skill name, or --all.", + "errors.skills.autoTrigger.unknownSkill": + "Unknown bundled skill: {value}. Supported skills: {skills}.", + "errors.skills.autoTrigger.publishFailed": + "Saved the setting, but {count} skill target(s) could not be republished: {targets}. Run `oo skills repair {repairArguments}` to finish applying it.", + "skills.autoTrigger.off.success.all": + "Every bundled skill is now manual-only.", + "skills.autoTrigger.off.success.skills": + "{count} bundled skill(s) are now manual-only: {skills}", + "skills.autoTrigger.on.success.all": + "Every bundled skill can auto-trigger again.", + "skills.autoTrigger.on.success.skills": + "{count} bundled skill(s) can auto-trigger again: {skills}", + "skills.autoTrigger.standingPolicyNote": + " Note: every bundled skill stays manual-only until `oo skills auto-trigger on --all` lifts the standing policy.", + "skills.autoTrigger.publishedLine": + " republished {count} skill target(s) across {agents} agent(s)", + "skills.autoTrigger.skippedLine": + " skipped {count} target(s) not managed by oo: {targets}", + "skills.autoTrigger.status.headerAll": + "Auto-trigger is off for every bundled skill.", + "skills.autoTrigger.status.headerSkills": + "Auto-trigger is off for {count} bundled skill(s).", + "skills.autoTrigger.status.headerDefault": + "Auto-trigger is on for every bundled skill.", + "skills.autoTrigger.status.line": " {name}: {state}", + "skills.autoTrigger.state.all": "manual (all)", + "skills.autoTrigger.state.default": "auto", + "skills.autoTrigger.state.skill": "manual", "commands.skills.recommend.description": "Plan end-of-session skill suggestions for the bundled oo skill and manage which packages are never suggested.", "commands.skills.recommend.summary": "Manage skill suggestions", @@ -2158,6 +2210,58 @@ export const zhMessages = { "skills.checkUpdate.failuresHeader": "失败：", "skills.checkUpdate.failuresLine": " {skillId}：{message}", + "commands.skills.autoTrigger.description": + "控制 agent 能否自行加载内置 skill。关闭自动触发后 skill 仍然安装着、仍可按名调用，同时会重新发布内置 skill，使改动对每个 agent 生效。", + "commands.skills.autoTrigger.summary": "控制内置 skill 的自动触发", + "commands.skills.autoTrigger.off.description": + "把指定的内置 skill 改为仅手动触发；使用 --all 把所有内置 skill 改为仅手动触发，包括后续版本新增的。", + "commands.skills.autoTrigger.off.summary": "关闭自动触发", + "commands.skills.autoTrigger.on.description": + "重新允许 agent 自行加载指定的内置 skill；使用 --all 把所有内置 skill 恢复为默认。", + "commands.skills.autoTrigger.on.summary": "开启自动触发", + "commands.skills.autoTrigger.status.description": + "显示每个内置 skill 的自动触发状态。", + "commands.skills.autoTrigger.status.summary": "显示自动触发状态", + "arguments.skills.autoTrigger.off.skillName": + "要改为仅手动触发的内置 skill 名称（可指定多个）", + "arguments.skills.autoTrigger.on.skillName": + "要恢复自动触发的内置 skill 名称（可指定多个）", + "options.skills.autoTrigger.off.all": + "把所有内置 skill 改为仅手动触发，而非指定的 skill", + "options.skills.autoTrigger.on.all": + "把所有内置 skill 恢复为默认，而非指定的 skill", + "errors.skills.autoTrigger.conflictingScope": + "请传入内置 skill 名称或 --all，不能同时使用。", + "errors.skills.autoTrigger.missingScope": + "请至少传入一个内置 skill 名称，或使用 --all。", + "errors.skills.autoTrigger.unknownSkill": + "未知的内置 skill：{value}。支持的 skill：{skills}。", + "errors.skills.autoTrigger.publishFailed": + "设置已保存，但有 {count} 个 skill 目标未能重新发布：{targets}。请运行 `oo skills repair {repairArguments}` 完成应用。", + "skills.autoTrigger.off.success.all": + "所有内置 skill 现在仅手动触发。", + "skills.autoTrigger.off.success.skills": + "{count} 个内置 skill 现在仅手动触发：{skills}", + "skills.autoTrigger.on.success.all": + "所有内置 skill 已恢复自动触发。", + "skills.autoTrigger.on.success.skills": + "{count} 个内置 skill 已恢复自动触发：{skills}", + "skills.autoTrigger.standingPolicyNote": + " 注意：在 `oo skills auto-trigger on --all` 解除全局策略之前，所有内置 skill 仍然仅手动触发。", + "skills.autoTrigger.publishedLine": + " 已在 {agents} 个 agent 上重新发布 {count} 个 skill 目标", + "skills.autoTrigger.skippedLine": + " 跳过 {count} 个非 oo 管理的目标：{targets}", + "skills.autoTrigger.status.headerAll": + "所有内置 skill 的自动触发已关闭。", + "skills.autoTrigger.status.headerSkills": + "有 {count} 个内置 skill 的自动触发已关闭。", + "skills.autoTrigger.status.headerDefault": + "所有内置 skill 的自动触发均已开启。", + "skills.autoTrigger.status.line": " {name}：{state}", + "skills.autoTrigger.state.all": "手动（全部关闭）", + "skills.autoTrigger.state.default": "自动", + "skills.autoTrigger.state.skill": "手动", "commands.skills.recommend.description": "为内置 oo skill 规划收尾阶段的 skill 推荐，并管理哪些包永不推荐。", "commands.skills.recommend.summary": "管理 skill 推荐", From 4d07cdd262163601142c5191786abcfc1d1df8fc Mon Sep 17 00:00:00 2001 From: Kevin Cui Date: Wed, 29 Jul 2026 06:47:44 -0400 Subject: [PATCH 2/2] fix(skills): decouple registry startup sync from the policy read Awaiting `readSkillAutoTriggerPolicy()` while the caller built its `Promise.all` array meant a settings file the run could not parse stopped `synchronizeRegistrySkills()` from ever being called. `FileSettingsStore.read()` throws `errors.store.readFailed` on a corrupt file, and registry synchronization has nothing to do with the auto-trigger policy, so one unreadable file took down both halves of startup synchronization instead of one. The read moves into `synchronizeBundledSkills()`, which is the only half that needs it. Also assert the exit code of the `auto-trigger off` setup runs in two tests whose expectations *are* the resulting state. Both would have stayed green if setup had silently failed, proving nothing. Signed-off-by: Kevin Cui --- src/application/commands/skills/auto-sync.ts | 15 +++++++++------ .../skills/auto-trigger/index.cli.test.ts | 11 +++++++++-- .../commands/skills/install.cli.test.ts | 12 +++++++++++- 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/application/commands/skills/auto-sync.ts b/src/application/commands/skills/auto-sync.ts index ab3797a..308ab36 100644 --- a/src/application/commands/skills/auto-sync.ts +++ b/src/application/commands/skills/auto-sync.ts @@ -57,11 +57,7 @@ export async function synchronizeManagedSkillsForAvailableHosts( } await Promise.all([ - synchronizeBundledSkills( - hosts, - context, - await readSkillAutoTriggerPolicy(context.settingsStore), - ), + synchronizeBundledSkills(hosts, context), synchronizeRegistrySkills(hosts, context), ]); } @@ -75,11 +71,18 @@ export async function synchronizeManagedSkillsForAvailableHosts( } } +// The policy is read here rather than by the caller so that a settings file +// this run cannot parse fails only the bundled half. Registry synchronization +// does not depend on the policy, and awaiting the read while the caller builds +// its `Promise.all` array would stop it from ever being started. async function synchronizeBundledSkills( hosts: readonly ManagedSkillHost[], context: SkillSyncContext, - autoTriggerPolicy: SkillAutoTriggerPolicy, ): Promise { + const autoTriggerPolicy = await readSkillAutoTriggerPolicy( + context.settingsStore, + ); + await Promise.all( hosts.flatMap(host => availableBundledSkillNames.map(skillName => diff --git a/src/application/commands/skills/auto-trigger/index.cli.test.ts b/src/application/commands/skills/auto-trigger/index.cli.test.ts index a3fcb91..66909d3 100644 --- a/src/application/commands/skills/auto-trigger/index.cli.test.ts +++ b/src/application/commands/skills/auto-trigger/index.cli.test.ts @@ -108,8 +108,15 @@ describe("skills auto-trigger CLI", () => { const sandbox = await createCliSandbox(); try { - await sandbox.run(["skills", "auto-trigger", "off", "oo-create-skill"]); - await sandbox.run(["skills", "auto-trigger", "off", "--all"]); + // Both setup runs are asserted: this test's expectations are the + // cleared state, so setup that silently failed would leave it green + // without ever having built the state it claims to clear. + for (const argv of [ + ["skills", "auto-trigger", "off", "oo-create-skill"], + ["skills", "auto-trigger", "off", "--all"], + ]) { + expect((await sandbox.run(argv)).exitCode).toBe(0); + } const result = await sandbox.run( ["skills", "auto-trigger", "on", "--all", "--json"], diff --git a/src/application/commands/skills/install.cli.test.ts b/src/application/commands/skills/install.cli.test.ts index c28709a..0d1b9c4 100644 --- a/src/application/commands/skills/install.cli.test.ts +++ b/src/application/commands/skills/install.cli.test.ts @@ -505,7 +505,17 @@ describe("skills install --out-dir export", () => { const outDir = await mkdtemp(join(tmpdir(), "oo-out-")); try { - await sandbox.run(["skills", "auto-trigger", "off", "--all"]); + // Asserted rather than fire-and-forget: this test proves the export + // ignores the local policy, so a setup run that silently failed + // would leave it green while proving nothing. + const disableResult = await sandbox.run([ + "skills", + "auto-trigger", + "off", + "--all", + ]); + + expect(disableResult.exitCode).toBe(0); sandbox.env.OO_SKILLS_SYNC_DISABLED = "1"; const result = await sandbox.run(["skills", "add", "--out-dir", outDir]);