diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 236e9b64a..db36a6118 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -343,7 +343,7 @@ The primary director is **dispatch** (`buildChatRole` → `createDispatchSystemP **Provider-conditional residuals.** Per-family additions layer on top of the shared block via the same `ModelFamilyPolicy` mechanism the directors use (`src/subagent/provider-family.ts`, `src/agent/model-family-policy.ts`) — additive lines, never prompt forks. **Grok** leaves get `buildGrokLeafAntiThrashNote` (gated by `shouldApplyGrokAntiThrash` / `applyGrokFinishBias`, withheld from orchestrators): a compact finish-bias reinforcement plus a one-line reminder to route file/web work through the dedicated tools rather than `run_shell`, motivated by observed tool-routing thrash on the same harness. **Kimi** intentionally has no residual yet — `detectModelFamily` already resolves the family so callers can branch on it, but the prompt seam is left unfilled pending eval characterization of Kimi's behavior, mirroring the provisional (permissive-default) policy in `model-family-policy.ts`. -`buildChatSystemPrompt` (TUI chat) assembles: base → core tool list → name-only skills listing → live `` block → appended extensions. `buildSubAgentSystemPrompt` assembles the worker prompt without a catalog skills listing. Closed directors that declare `attachedSkills` (none of the shipped directors do today; never the primary) get those bodies injected once at spawn from **plugin skill dirs only** (`skillDirsFromEnabledPlugins`) — `resolveSkillBody` is called with `pluginDirsOnly`, so a project-local `.agents`/`.claude`/`.codex/skills` SKILL.md cannot become system-prompt constraints. A miss is noted in the attached section; the worker proceeds (no park). Optional skills stay names-only in the identity header; workers mount `skill_search` + `use_skill` on every family (including grok/kimi leaves), scoped to the union of `attachedSkills` and `optionalSkills`. Built-in catalog tools (including `skill_search`) are advertised on the primary wire and callable directly; MCP integrations are discovered via `tool_search` rather than being enumerated. Skills follow the same lazy principle on the primary: each discovered skill contributes only its name. The model calls `skill_search` for descriptions when choosing, then `use_skill` to load a body. The operator can also invoke the same skill as `/` (see Skills below). Skills are discovered (and deduped by name, first-wins) from enabled plugin dirs, then `.agents`/`.claude`/`.codex/skills`, in that precedence. Corbits Code ships a bundled catalog via the first-party `corbits-skills` plugin (origin `repo`); project-local skills of the same name are shadowed by an enabled plugin skill. +`buildChatSystemPrompt` (TUI chat) assembles: base → core tool list → name-only skills listing → live `` block → appended extensions. `buildSubAgentSystemPrompt` assembles the worker prompt without a catalog skills listing. Closed directors that declare `attachedSkills` (none of the shipped directors do today; never the primary) get those bodies injected once at spawn from **plugin skill dirs only** (`skillDirsFromEnabledPlugins`) — `resolveSkillBody` is called with `pluginDirsOnly`, so a project-local `.corbits`/`.agents`/`.claude`/`.codex/skills` SKILL.md cannot become system-prompt constraints. A miss is noted in the attached section; the worker proceeds (no park). Optional skills stay names-only in the identity header; workers mount `skill_search` + `use_skill` on every family (including grok/kimi leaves), scoped to the union of `attachedSkills` and `optionalSkills`. A worker that declares neither field gets an empty allowlist (fail closed) — `skill_search` / `use_skill` match nothing until the package or plugin profile lists names. `coder` ships `optionalSkills: ["typescript"]`. Built-in catalog tools (including `skill_search`) are advertised on the primary wire and callable directly; MCP integrations are discovered via `tool_search` rather than being enumerated. Skills follow the same lazy principle on the primary: each discovered skill contributes only its name. The model calls `skill_search` for descriptions when choosing, then `use_skill` to load a body. The operator can also invoke the same skill as `/` (see Skills below). Skills are discovered (and deduped by name, first-wins) from enabled plugin dirs, then project `.corbits`/`.agents`/`.claude`/`.codex/skills`, then the same relative dirs under `homedir()`. A user-global path that equals the project path (cwd is home) is skipped. Corbits Code ships a bundled catalog via the first-party `corbits-skills` plugin (origin `repo`); project-local skills of the same name are shadowed by an enabled plugin skill. A trusted/enabled plugin (or local `.agents/agents/`) profile whose `id` matches a closed director replaces that director at `spawn_agent(agent=…)` (prompt, capabilities, inference, skill fields); `intent=` still resolves the closed package. **Overrides.** `loadSystemPromptOverrides` (`src/agent/context-extensions.ts`) resolves a project `SYSTEM.md` (repo root, then `.corbits/`) that **replaces** the static base block, and an `APPEND_SYSTEM.md` that is **appended** as an extension. These compose with `config.systemPromptExtensions` (profile config) and the auto-discovered `AGENTS.md`, all of which attach as appended sections after the base. @@ -455,12 +455,17 @@ The primary is the dispatch director. Bundled skill bodies are **how-to playbook `discoverSkills(cwd, pluginDirs)` runs at session start and returns each available skill's `name` + one-line `description`. The system prompt listing is names only; descriptions feed `skill_search` (and the slash picker). It scans the following base directories, highest precedence first: -| Base directory | Source | -| --------------------- | --------------------------------------------------------------------------------------------------------------------------------------- | -| `/skills/` | Each enabled plugin that ships skills, including the bundled `corbits-skills` catalog when auto-enabled (`skillDirsFromEnabledPlugins`) | -| `.agents/skills/` | Shared across runtimes | -| `.claude/skills/` | Claude Code workspace skills | -| `.codex/skills/` | Codex workspace skills | +| Base directory | Source | +| ---------------------------- | --------------------------------------------------------------------------------------------------------------------------------------- | +| `/skills/` | Each enabled plugin that ships skills, including the bundled `corbits-skills` catalog when auto-enabled (`skillDirsFromEnabledPlugins`) | +| `/.corbits/skills/` | Project-local Corbits skills | +| `/.agents/skills/` | Shared across runtimes | +| `/.claude/skills/` | Claude Code workspace skills | +| `/.codex/skills/` | Codex workspace skills | +| `/.corbits/skills/` | User-global Corbits skills (skipped when the path equals the project path) | +| `/.agents/skills/` | User-global shared skills | +| `/.claude/skills/` | User-global Claude skills | +| `/.codex/skills/` | User-global Codex skills | Each `//SKILL.md` is one skill. Discovery dedupes by directory name: the first base dir that provides a given name wins, so an enabled plugin skill shadows a project-local skill of the same name. Plugin dirs are passed in discovery order (repo first), so a first-party catalog name wins over a later marketplace or project skill of the same name. Skills with `disable-model-invocation: true` are omitted from the returned listing but still claim the name (first-wins), so a lower-priority same-name skill cannot leak into the listing. `resolveSkillBody(cwd, ref, pluginDirs)` resolves a skill's body using the same ordered list (it accepts a bare name or a `plugin:name` ref, keying on the name) and **does not** hard-fail on `disable-model-invocation` — explicit `use_skill("name")` still loads background libraries. @@ -481,10 +486,10 @@ There is no skill `type` field required for model invocation — a skill body is `buildSkillsSection` lists discovered skill names in the system prompt (no descriptions). Details come from `skill_search`; the full instructions enter context in two ways: -1. **Model** — `skill_search` for descriptions, then `use_skill` (`src/agent/use-skill.ts`) with a skill name. The handler refuses names already attached at spawn or already loaded this session (short “already in context”; it does not dump the body again). Otherwise it calls `resolveSkillBody`, strips the frontmatter, and returns the body as the tool result. +1. **Model** — `skill_search` for descriptions, then `use_skill` (`src/agent/use-skill.ts`) with a skill name. The handler refuses names already attached at spawn or already loaded this session (short “already in context”; it does not dump the body again). Otherwise it calls `resolveSkillBody`, strips the frontmatter, and returns the body as the tool result. The result appends a `Skill directory:` footer naming the winning skill dir plus a sibling-file hint (read sibling files with `read_file` relative to that dir). 2. **Operator** — `/` from `loadSkillCommands` sends the same SKILL.md body (plus typed args) to the primary as a user turn. Skills with `user-invocable: false` are omitted from the slash registry and remain `use_skill` only. Dispatch then follows the recipe. -Which plugin skill directories are in scope is decided in `runner.ts` / `skillDirsFromEnabledPlugins`, which passes the enabled plugins' dirs to both `discoverSkills` (for the listing) and the `use_skill` tool (for resolution). Project-local `.agents`/`.claude`/`.codex/skills` are always searched. Slash-command registration is first-wins (built-ins, then plugins in discovery order), so a first-party `/implement` stays first-party if a marketplace plugin of the same slash name is also enabled. +Which plugin skill directories are in scope is decided in `runner.ts` / `skillDirsFromEnabledPlugins`, which passes the enabled plugins' dirs to both `discoverSkills` (for the listing) and the `use_skill` tool (for resolution). Project-local `.corbits`/`.agents`/`.claude`/`.codex/skills` and the same dirs under `homedir()` are always searched (user paths that equal project paths are skipped). Slash-command registration is first-wins (built-ins, then plugins in discovery order), so a first-party `/implement` stays first-party if a marketplace plugin of the same slash name is also enabled. ## Data Flow diff --git a/docs/IMPLEMENTATION.md b/docs/IMPLEMENTATION.md index fde3f24a0..9e0c224f1 100644 --- a/docs/IMPLEMENTATION.md +++ b/docs/IMPLEMENTATION.md @@ -160,7 +160,7 @@ Twenty packages under `src/agent/directors//` register in `DIRECTOR_REGISTRY 1. `spawn_agent(agent=…)` / `spawn_agent(intent=…)` → `resolveDirector` in `agent-fleet.ts` before tools and system prompt are built. Bare `spawn_agent` (neither field) and `intent=general` fail closed. After director resolution, `createSpawnAgentTool` fail-closes implement/review (and their default directors) without non-empty `success_criteria`. 2. `packageToProfile` maps envelope (`tools.allow`/`deny`) to `AgentProfile.capabilities` and `spawn.maySpawn` → `orchestrator`. System prompts are prefixed with a stable identity block (`formatDirectorSystemPrompt`: agent id, model role, optional skills). 3. Nested spawn: packages with `spawn.allowlist` forward that list into nested `spawn_agent` (`spawnAllowlist` on nestedDispatch). Off-list `agent` is refused. `spawn_agent(agent=dispatch)` is refused (primary is not a spawned worker). Primary omits the list so plugin profiles stay reachable. -4. `directorProfiles()` is the spawn catalog (`default-agents.ts`) — closed set minus dispatch. Plugin and local `.agents/agents/` profiles still load, but closed `DIRECTOR_IDS` cannot be overridden or aliased. +4. `directorProfiles()` is the spawn catalog (`default-agents.ts`) — closed set minus dispatch. Plugin and local `.agents/agents/` profiles still load; a trusted/enabled plugin (or local file) whose `id` matches a closed director **replaces** that director at `spawn_agent(agent=…)` (prompt, capabilities, inference, skill allowlist). `intent=` still maps to the closed package with no overlay. 5. Primary chat role is dispatch: `buildChatRole()` → `createDispatchSystemPrompt()`. The dispatch card is a ~3–5k coordinator (classify, mechanical DIY, spawn named specialists); idle/mailbox/poll live in the harness, and family residuals come from `@corbits/prompt-variance`. Product mutation tools (`write_file` / `edit_file` / `delete_file`) live in CORE (and `DISPATCH_TOOLS`) so they are advertised on the primary without a `tool_search` round-trip. DIY is only for obvious mechanical corrections needing no diagnosis, design, new behavior, or new tests. Spawn coder/shakespeare or the relevant specialist for substantive work regardless of file count, including single-file bug fixes. Existing exploration is not an exemption. This is prompt policy, not a toolset strip. `PRIMARY_DENIED_PRODUCT_TOOLS` is gone. Shell file-writes stay denied; MCP tools are not re-filtered by a product-write deny list. There is no static per-profile write-path lock (CL-6952). **Grok infer envelope.** `loadSessionChatPrompt` always appends `loadAgentContextExtensions` (`AGENTS.md`, capped at `MAX_AGENTS_MD_BYTES`) and advertises CORE+CATALOG full schemas via `advertisedToolNamesForSessionMode`. That assembly is family-agnostic — Grok does not substitute the trimmed director prompt (`buildSubAgentSystemPrompt` + `formatDirectorSystemPrompt`). Workers already use that trimmed path (no `AGENTS.md`, mounted-tool schemas only). Keep the infer envelope on Grok; do not strip `AGENTS.md` or core schemas. Measure in `src/agent/prompt-sizes.ts` (`assembleDirectorPrompt`). @@ -168,7 +168,7 @@ Twenty packages under `src/agent/directors//` register in `DIRECTOR_REGISTRY **One advertised posix set (CL-8400).** Registry engines stay posix-named (`read_file`, `write_file`, `edit_file`, `delete_file`, `run_shell`, `search_files`, `grep`). Advertise is a 1:1 projection onto wire names (`read`, `write`, `edit`, `delete`, `bash`, `glob`, `grep`) plus the unchanged control-plane. Incoming aliases (wire names, old posix ids, Codex `shell` → `run_shell`, `update_plan` → `manage_tasks`) canonicalize onto the engine id for dispatch and grants. `apply_patch` is neither advertised nor dispatched (not an alias of `edit`). `list_dir` stays mounted and unadvertised. Director `tools.allow` stays engine names. Codex does not dual-publish `shell`+`run_shell`. Hidden `shell` coerces Codex `command` (string or `["bash","-lc",script]` argv), `workdir`, and `timeout_ms` onto `run_shell`. Hidden `update_plan` maps `plan: [{step, status}]` onto `manage_tasks(action: "create")` (`pending`/`in_progress`/`completed` → `todo`/`doing`/`done`). 6. There is no static write-path declaration on packages or profiles (CL-6952 removed it — no shipped director ever set one). Instead, `agent-fleet.ts` tracks each running dispatch by cwd; a new mutating dispatch that lands on the same cwd as a live mutating peer (`pending_init`/`running`, and not a declared read-only `modelRole` of `explore`/`plan`/`review`/`test`) records at most one `concurrent-lane-overlap` entry per cwd wave in `intervention-log.ts` (class `conflict`). The wave flag clears when no live mutating writer remains for that cwd. Terminal-but-unsettled lanes (for example cancelled with `finishedAt` set while the run promise has not reached `finally`) are pruned from the map and do not warn. This is advisory only — it never blocks the spawn, since cwd overlap does not prove the two lanes touch the same files. -7. Spawn effort: pin > package `modelRole` default (`defaultEffortForDirector`; plan/review/orchestrator=high; implement/explore/docs/test=medium) > orchestrator/worker binary > parent inheritance. Attached skills (none on the shipped directors today; never the primary) are injected into the worker system prompt at spawn from plugin skill dirs only (no project-local `.agents`/`.claude`/`.codex` fallback). Optional skills are listed in the identity header for awareness; workers mount `skill_search` + `use_skill` on every family, scoped to the union of `attachedSkills` and `optionalSkills`. `use_skill` refuses names already attached or already loaded this session and does not return the body again. Primary mounts `use_skill` for its own skill list (same in-session refuse; no attached set). +7. Spawn effort: pin > package `modelRole` default (`defaultEffortForDirector`; plan/review/orchestrator=high; implement/explore/docs/test=medium) > orchestrator/worker binary > parent inheritance. Attached skills (none on the shipped directors today; never the primary) are injected into the worker system prompt at spawn from plugin skill dirs only (no project-local `.corbits`/`.agents`/`.claude`/`.codex` or user-global fallback). Optional skills are listed in the identity header for awareness; workers mount `skill_search` + `use_skill` on every family, scoped to the union of `attachedSkills` and `optionalSkills`. A worker that declares neither field gets an empty allowlist (fail closed). `coder` lists `typescript`. `use_skill` refuses names already attached or already loaded this session and does not return the body again. Primary mounts `use_skill` for its own skill list (same in-session refuse; no attached set). `spawn_agent` accepts a `skills[]` param unioned with the package allowlist for the worker's `skill_search` / `use_skill` scope. Plugin skill discovery also searches `/.corbits/skills`, `homedir()/.corbits/skills`, `homedir()/.agents/skills`, `homedir()/.claude/skills`, and `homedir()/.codex/skills`. Intent defaults: `intent=implement` → director `coder`; `explore` → `explorer`; `plan` → `planner`; `review` → `reviewer`; general → error. Spawn: dispatch full fleet; all other directors mount no fleet tools. Dispatch assigns one focused task per worker; fan-out width follows independent lanes (one lane per PR/path/ownership). Live `` injects cwd, platform, arch, runtime, date, and git status on every chat and worker prompt. diff --git a/docs/PLUGINS.md b/docs/PLUGINS.md index 5ddfe255e..a6d401545 100644 --- a/docs/PLUGINS.md +++ b/docs/PLUGINS.md @@ -272,18 +272,21 @@ shape. - **Skill refs on agent frontmatter / body.** A skill name is either: - **Bare** — `typescript`, `plan`, or a namespaced `plugin:typescript`. Resolved by searching the plugin's `skills/` dir first, then project-local fallbacks - (`.agents/skills`, `.claude/skills`, `.codex/skills`). Prefer bare names for + (`.corbits/skills`, `.agents/skills`, `.claude/skills`, `.codex/skills`), + then the same relative dirs under `homedir()`. Prefer bare names for co-located skills; they are the portable, discoverable form. - **Path-like** — `./skills/typescript`, `skills/typescript`, `../sibling-skill`, or any ref containing `/` (including a trailing `SKILL.md`). Resolved only under the plugin root (`pluginRoot`), with lexical containment plus a realpath check so a symlink under the root cannot escape. Absolute paths, bare `.` / `..`, and escapes outside the root are rejected (skill miss, not a crash). +- `spawn_agent` accepts a `skills[]` param unioned with the package allowlist, scoping the worker's `skill_search` / `use_skill`. - An agent plugin is wired in only when `settings.plugins[id].enabled` is true (same gating as command plugins — no consent needed, since profiles are configuration data, not in-process code). - Profile precedence: built-in defaults < plugin profiles < local - `.agents/agents/*.json` (most specific wins on same-id conflicts). + `.agents/agents/*.json` (most specific wins on same-id conflicts, including + closed director ids — a plugin `id: designer` replaces the shipped designer). - Per-kind verify in `/plugins` (agent = profile count check). - Add-by-path (Alt+A) uses the same path suggestion UX as `@` mentions (`listPathSuggestions`) so registering a plugin from disk can browse directories. diff --git a/src/agent/default-agents.ts b/src/agent/default-agents.ts index cfa88f95c..fedaec346 100644 --- a/src/agent/default-agents.ts +++ b/src/agent/default-agents.ts @@ -2,8 +2,8 @@ import { directorProfiles } from "./directors/registry.js"; import type { AgentPlugin } from "./profile-types.js"; // Spawnable profiles = closed director fleet minus primary dispatch. -// Closed DIRECTOR_IDS are reserved: plugin/local profiles that collide are -// skipped at load (CL-7015) — no override or alias of the fleet. +// Plugin/local profiles with the same id replace the closed director +// (CL-9917) — last enabled plugin / local file wins. export const defaultAgentsPlugin: AgentPlugin = { agents: directorProfiles(), }; diff --git a/src/agent/directors/coder/package.ts b/src/agent/directors/coder/package.ts index c6370ab42..7460f8857 100644 --- a/src/agent/directors/coder/package.ts +++ b/src/agent/directors/coder/package.ts @@ -23,6 +23,7 @@ export const coderPackage: DirectorPackage = { spawn: { maySpawn: false }, tier: "leaf", modelRole: "implement", + optionalSkills: ["typescript"], systemPrompt: `You are CoderDirector (Coder), a specialist in Corbits Code. PRIMARY INTENT: implement the brief in product code. Edit, verify, report. diff --git a/src/agent/directors/identity.test.ts b/src/agent/directors/identity.test.ts index 4754026c2..271ef5525 100644 --- a/src/agent/directors/identity.test.ts +++ b/src/agent/directors/identity.test.ts @@ -26,14 +26,20 @@ describe("formatDirectorSystemPrompt", () => { }); describe("packageAllowedSkillNames", () => { - test("returns undefined for universal skill access when unconfigured", () => { - expect(packageAllowedSkillNames(DIRECTOR_REGISTRY.coder)).toBeUndefined(); - expect( - packageAllowedSkillNames(DIRECTOR_REGISTRY.dispatch), - ).toBeUndefined(); - expect( - packageAllowedSkillNames(DIRECTOR_REGISTRY.explorer), - ).toBeUndefined(); + test("returns empty allowlist when neither field is declared", () => { + expect(packageAllowedSkillNames(undefined)).toBeUndefined(); + expect(packageAllowedSkillNames(DIRECTOR_REGISTRY.dispatch)).toEqual([]); + expect(packageAllowedSkillNames(DIRECTOR_REGISTRY.explorer)).toEqual([]); + }); + + test("coder optionalSkills is typescript", () => { + expect(packageAllowedSkillNames(DIRECTOR_REGISTRY.coder)).toEqual([ + "typescript", + ]); + }); + + test("designer with no skill fields is fail-closed", () => { + expect(packageAllowedSkillNames(DIRECTOR_REGISTRY.designer)).toEqual([]); }); test("unions attached then optional without duplicating when configured", () => { diff --git a/src/agent/directors/identity.ts b/src/agent/directors/identity.ts index ae2d6c1f4..6b56ae941 100644 --- a/src/agent/directors/identity.ts +++ b/src/agent/directors/identity.ts @@ -48,22 +48,17 @@ export function formatDirectorSystemPrompt(pkg: DirectorPackage): string { /** * Allowlist for a worker's skill_search + use_skill: union of attached and - * optional names, first-wins, order-preserving. Undefined when the package - * declares neither field (plugin profiles / directors with no skill scope). + * optional names, first-wins, order-preserving. Empty when neither field is + * declared so workers fail closed (search/use_skill match nothing). */ -export function packageAllowedSkillNames( - pkg: DirectorPackage | undefined, -): readonly string[] | undefined { - if (pkg === undefined) return undefined; - if (pkg.attachedSkills === undefined && pkg.optionalSkills === undefined) { - return undefined; - } +export function skillNamesFromFields( + attached: readonly string[] | undefined, + optional: readonly string[] | undefined, +): readonly string[] { + if (attached === undefined && optional === undefined) return []; const seen = new Set(); const names: string[] = []; - for (const name of [ - ...(pkg.attachedSkills ?? []), - ...(pkg.optionalSkills ?? []), - ]) { + for (const name of [...(attached ?? []), ...(optional ?? [])]) { if (seen.has(name)) continue; seen.add(name); names.push(name); @@ -71,6 +66,18 @@ export function packageAllowedSkillNames( return names; } +/** + * Allowlist for a director package. Undefined only when `pkg` itself is + * undefined (plugin path handles skill fields separately). A defined package + * with neither skill field yields `[]`, not the full catalog. + */ +export function packageAllowedSkillNames( + pkg: DirectorPackage | undefined, +): readonly string[] | undefined { + if (pkg === undefined) return undefined; + return skillNamesFromFields(pkg.attachedSkills, pkg.optionalSkills); +} + /** * Product default reasoning effort by package modelRole (CL-5816 slice). */ diff --git a/src/agent/directors/registry.test.ts b/src/agent/directors/registry.test.ts index 22a9d3480..f3c3ff5a5 100644 --- a/src/agent/directors/registry.test.ts +++ b/src/agent/directors/registry.test.ts @@ -126,6 +126,13 @@ describe("director registry", () => { expect(c.spawn.allowlist).toBeUndefined(); expect(c.tier).toBe("leaf"); expect(packageToProfile(c).orchestrator).toBe(false); + expect(c.optionalSkills).toEqual(["typescript"]); + expect(packageToProfile(c).optionalSkills).toEqual(["typescript"]); + }); + + test("designer does not ship third-party skill names", () => { + expect(DIRECTOR_REGISTRY.designer.optionalSkills).toBeUndefined(); + expect(DIRECTOR_REGISTRY.designer.attachedSkills).toBeUndefined(); }); test("dispatch is the only maySpawn:true closed director", () => { diff --git a/src/agent/directors/registry.ts b/src/agent/directors/registry.ts index f2f338a8f..75f5b9862 100644 --- a/src/agent/directors/registry.ts +++ b/src/agent/directors/registry.ts @@ -131,6 +131,12 @@ export function packageToProfile(pkg: DirectorPackage): AgentProfile { // Dispatch maySpawn marks intent; leaves stay non-orchestrator. orchestrator: pkg.spawn.maySpawn, ...(capabilities !== undefined ? { capabilities } : {}), + ...(pkg.attachedSkills !== undefined + ? { attachedSkills: [...pkg.attachedSkills] } + : {}), + ...(pkg.optionalSkills !== undefined + ? { optionalSkills: [...pkg.optionalSkills] } + : {}), }; } diff --git a/src/agent/profile-types.ts b/src/agent/profile-types.ts index 1bd921e44..161954f9a 100644 --- a/src/agent/profile-types.ts +++ b/src/agent/profile-types.ts @@ -66,6 +66,12 @@ export interface AgentProfile { // rejects profile orchestrators because only built-in director packages carry // trusted fleet semantics. Leaf workers should leave this unset. orchestrator?: boolean; + // Skill names whose bodies are injected once at spawn. Same contract as + // DirectorPackage.attachedSkills. + attachedSkills?: string[]; + // Optional skill names the worker may skill_search / use_skill. When both + // this and attachedSkills are omitted, the worker allowlist is empty. + optionalSkills?: string[]; // Where the profile came from, for search_agents labeling (e.g. "claude", // "plugin:", "local"). Omitted for built-in defaults. source?: string; diff --git a/src/agent/profiles.test.ts b/src/agent/profiles.test.ts new file mode 100644 index 000000000..db354bae5 --- /dev/null +++ b/src/agent/profiles.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, test } from "bun:test"; +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { loadAgentProfiles } from "./profiles.js"; +import { defined } from "../../testkit/defined.js"; + +describe("loadAgentProfiles director overlay", () => { + test("plugin extraProfiles with a closed director id replace the default", async () => { + const dir = await mkdtemp(join(tmpdir(), "cl9917-profiles-")); + try { + const profiles = await loadAgentProfiles(dir, [ + { + id: "designer", + source: "plugin:p1", + systemPromptRole: "Plugin designer prompt.", + optionalSkills: ["better-ui"], + }, + ]); + const designer = defined( + profiles.find((p) => p.id === "designer"), + "designer profile", + ); + expect(designer.systemPromptRole).toBe("Plugin designer prompt."); + expect(designer.source).toBe("plugin:p1"); + expect(designer.optionalSkills).toEqual(["better-ui"]); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + test("local file with a closed director id replaces the default", async () => { + const dir = await mkdtemp(join(tmpdir(), "cl9917-profiles-local-")); + try { + await writeFile( + join(dir, "designer.json"), + JSON.stringify({ + id: "designer", + systemPromptRole: "Local designer prompt.", + }), + ); + const profiles = await loadAgentProfiles(dir); + const designer = defined( + profiles.find((p) => p.id === "designer"), + "designer profile", + ); + expect(designer.systemPromptRole).toBe("Local designer prompt."); + expect(designer.source).toBe("local"); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); +}); diff --git a/src/agent/profiles.ts b/src/agent/profiles.ts index cb2135e1b..10fce7873 100644 --- a/src/agent/profiles.ts +++ b/src/agent/profiles.ts @@ -4,7 +4,6 @@ import { join } from "node:path"; import { type } from "arktype"; import { defaultAgentsPlugin as defaultPlugin } from "./default-agents.js"; -import { isDirectorId } from "./directors/registry.js"; import { REASONING_EFFORTS } from "./profile-types.js"; export type { @@ -54,6 +53,8 @@ const AgentProfileSchema = type({ "systemPromptRole?": "string", "systemPromptPath?": "string", "orchestrator?": "boolean", + "attachedSkills?": "string[]", + "optionalSkills?": "string[]", }); function isENOENT(err: unknown): boolean { @@ -67,8 +68,8 @@ function isENOENT(err: unknown): boolean { // Mutable registry seeded with the default plugin. Plugin-provided profiles // are overridable: a profile with the same id loaded later (or from the local -// .agents/agents/ directory) replaces the earlier one — except closed -// DIRECTOR_IDS, which are reserved and skipped at load (CL-7015). +// .agents/agents/ directory) replaces the earlier one, including closed +// director ids — last enabled plugin / local file wins (CL-9917). const registry: AgentProfile[] = [...defaultPlugin.agents]; // Load diagnostics: additive over loadAgentProfiles. `revision` stamps the @@ -104,18 +105,12 @@ function mergeProfileInto(list: AgentProfile[], profile: AgentProfile): void { else list.push(profile); } -/** Skip profiles whose id collides with a closed director (no override/alias). */ -function isReservedDirectorProfile(profile: AgentProfile): boolean { - return isDirectorId(profile.id); -} - // Load and merge profiles from three sources, in ascending precedence: // 1. The built-in default registry // 2. `extraProfiles` — profiles contributed by enabled agent-kind plugins // 3. JSON/YAML files in the local .agents/agents/ directory // A profile with a duplicate id loaded from a higher-precedence source replaces -// the earlier one. Closed DIRECTOR_IDS are reserved: colliding plugin/local -// profiles are skipped so the fleet cannot be overridden or aliased. +// the earlier one, including closed director ids (plugin/local overlay wins). export async function loadAgentProfiles( dir: string, extraProfiles: AgentProfile[] = [], @@ -148,10 +143,7 @@ export async function loadAgentProfilesWithDiagnostics( } catch (err) { if (isENOENT(err)) { const merged = [...registry]; - for (const p of extraProfiles) { - if (isReservedDirectorProfile(p)) continue; - mergeProfileInto(merged, p); - } + for (const p of extraProfiles) mergeProfileInto(merged, p); return done(merged); } throw err; @@ -187,7 +179,7 @@ export async function loadAgentProfilesWithDiagnostics( continue; } const profile = result as AgentProfile; - if (isReservedDirectorProfile(profile)) continue; + if (profile.source === undefined) profile.source = "local"; // Resolve systemPromptPath relative to this directory. The file content // becomes systemPromptRole; an explicit systemPromptRole takes precedence. if ( @@ -208,10 +200,7 @@ export async function loadAgentProfilesWithDiagnostics( } const merged = [...registry]; - for (const profile of extraProfiles) { - if (isReservedDirectorProfile(profile)) continue; - mergeProfileInto(merged, profile); - } + for (const profile of extraProfiles) mergeProfileInto(merged, profile); for (const profile of local) mergeProfileInto(merged, profile); return done(merged); } diff --git a/src/agent/use-skill.test.ts b/src/agent/use-skill.test.ts index 2765ca7b3..ac80f0dbe 100644 --- a/src/agent/use-skill.test.ts +++ b/src/agent/use-skill.test.ts @@ -57,11 +57,11 @@ describe("createUseSkillTool already-in-context", () => { import.meta.resolve("../extensions/skills.js"), (real: typeof import("../extensions/skills.js")) => ({ ...real, - resolveSkillBody: async ( - ...args: Parameters + resolveSkillWithDir: async ( + ...args: Parameters ) => { resolveCalls += 1; - return real.resolveSkillBody(...args); + return real.resolveSkillWithDir(...args); }, }), async () => { @@ -102,12 +102,12 @@ describe("createUseSkillTool already-in-context", () => { import.meta.resolve("../extensions/skills.js"), (real: typeof import("../extensions/skills.js")) => ({ ...real, - resolveSkillBody: async ( - ...args: Parameters + resolveSkillWithDir: async ( + ...args: Parameters ) => { resolveCalls += 1; await Promise.resolve(); - return real.resolveSkillBody(...args); + return real.resolveSkillWithDir(...args); }, }), async () => { @@ -144,3 +144,25 @@ describe("createUseSkillTool already-in-context", () => { expect(out).toContain('Skill "git-worktrees"'); }); }); + +describe("createUseSkillTool skill directory footer", () => { + test("loaded body appends the skill dir plus a sibling-file hint", async () => { + const cwd = await fixtureWithHiddenSkill(); + const tool = createUseSkillTool(cwd); + const out = await call(tool, { name: "git-worktrees" }); + expect(out).toContain("Create worktree recipe."); + expect(out).toContain( + `Skill directory: ${join(cwd, ".agents", "skills", "git-worktrees")}.`, + ); + expect(out).toContain("brand-guidelines.md"); + expect(out).toContain("read_file"); + }); + + test("unknown skill has no footer", async () => { + const cwd = await fixtureWithHiddenSkill(); + const tool = createUseSkillTool(cwd); + const out = await call(tool, { name: "nope-missing" }); + expect(out).toBe('No skill named "nope-missing" is available.'); + expect(out).not.toContain("Skill directory:"); + }); +}); diff --git a/src/agent/use-skill.ts b/src/agent/use-skill.ts index edaac0892..0e04e58d2 100644 --- a/src/agent/use-skill.ts +++ b/src/agent/use-skill.ts @@ -3,7 +3,7 @@ import type { AgentTool } from "@intx/agent"; import type { ToolDefinition } from "@intx/types/runtime"; import { type } from "arktype"; -import { resolveSkillBody } from "../extensions/skills.js"; +import { resolveSkillWithDir } from "../extensions/skills.js"; import { NOOP_TELEMETRY, type Telemetry } from "../telemetry/index.js"; import { captureSkillUsed } from "../telemetry/product-events.js"; @@ -72,8 +72,11 @@ export function createUseSkillTool( if (loaded.has(name)) return alreadyInContextMessage(name); loaded.add(name); let body: string | undefined; + let dir: string | undefined; try { - body = await resolveSkillBody(cwd, name, skillDirs); + const resolved = await resolveSkillWithDir(cwd, name, skillDirs); + body = resolved?.body; + dir = resolved?.dir; } catch (err) { loaded.delete(name); throw err; @@ -86,7 +89,11 @@ export function createUseSkillTool( // name never leaves the process: first-party `corbits-skills` names // are reported by name, everything else as `custom`. captureSkillUsed(telemetry, name); - return `Skill "${name}" — follow these instructions for this task:\n\n${body}`; + const footer = + dir !== undefined + ? `\n\nSkill directory: ${dir}. Sibling files (e.g. brand-guidelines.md) can be read with read_file relative to that dir.` + : ""; + return `Skill "${name}" — follow these instructions for this task:\n\n${body}${footer}`; }, }); } diff --git a/src/extensions/skills.test.ts b/src/extensions/skills.test.ts index 6002217ae..73da0aac7 100644 --- a/src/extensions/skills.test.ts +++ b/src/extensions/skills.test.ts @@ -3,8 +3,13 @@ import { join } from "node:path"; import { tmpdir } from "node:os"; import { test, expect, describe, beforeEach, afterEach } from "bun:test"; -import { discoverSkills, resolveSkillBody } from "./skills.js"; +import { + discoverSkills, + resolveSkillBody, + resolveSkillWithDir, +} from "./skills.js"; import { defined } from "../../testkit/defined.js"; +import { withMockedHomedir } from "../../testkit/mock-module.js"; const fixtureCwd = join(import.meta.dirname, "../../fixtures/skill-workspace"); const exampleAgentPlugin = join( @@ -80,6 +85,92 @@ describe("skill discovery", () => { await rm(plugin, { recursive: true, force: true }); } }); + + test("discovers a project .corbits/skills skill", async () => { + const cwd = await mkdtemp(join(tmpdir(), "skill-corbits-project-")); + try { + await mkdir(join(cwd, ".corbits", "skills", "local-playbook"), { + recursive: true, + }); + await writeFile( + join(cwd, ".corbits", "skills", "local-playbook", "SKILL.md"), + "---\nname: local-playbook\ndescription: project corbits skill\n---\nProject body.\n", + "utf8", + ); + const skills = await discoverSkills(cwd, []); + expect(skills.find((s) => s.name === "local-playbook")).toEqual({ + name: "local-playbook", + description: "project corbits skill", + }); + const body = await resolveSkillBody(cwd, "local-playbook", []); + expect(body).toContain("Project body."); + } finally { + await rm(cwd, { recursive: true, force: true }); + } + }); + + test("discovers a user-global ~/.corbits/skills skill (tmp HOME)", async () => { + const cwd = await mkdtemp(join(tmpdir(), "skill-corbits-cwd-")); + const home = await mkdtemp(join(tmpdir(), "skill-corbits-home-")); + try { + await mkdir(join(home, ".corbits", "skills", "user-playbook"), { + recursive: true, + }); + await writeFile( + join(home, ".corbits", "skills", "user-playbook", "SKILL.md"), + "---\nname: user-playbook\ndescription: user global skill\n---\nUser body.\n", + "utf8", + ); + await withMockedHomedir(home, async () => { + const skills = await discoverSkills(cwd, []); + expect(skills.find((s) => s.name === "user-playbook")).toEqual({ + name: "user-playbook", + description: "user global skill", + }); + const body = await resolveSkillBody(cwd, "user-playbook", []); + expect(body).toContain("User body."); + }); + } finally { + await rm(cwd, { recursive: true, force: true }); + await rm(home, { recursive: true, force: true }); + } + }); + + test("project .corbits/skills shadows the same name in user-global", async () => { + const cwd = await mkdtemp(join(tmpdir(), "skill-corbits-shadow-cwd-")); + const home = await mkdtemp(join(tmpdir(), "skill-corbits-shadow-home-")); + try { + await mkdir(join(cwd, ".corbits", "skills", "shared"), { + recursive: true, + }); + await mkdir(join(home, ".corbits", "skills", "shared"), { + recursive: true, + }); + await writeFile( + join(cwd, ".corbits", "skills", "shared", "SKILL.md"), + "---\nname: shared\ndescription: project wins\n---\nProject shared.\n", + "utf8", + ); + await writeFile( + join(home, ".corbits", "skills", "shared", "SKILL.md"), + "---\nname: shared\ndescription: user should lose\n---\nUser shared.\n", + "utf8", + ); + await withMockedHomedir(home, async () => { + const skills = await discoverSkills(cwd, []); + expect(skills.find((s) => s.name === "shared")).toEqual({ + name: "shared", + description: "project wins", + }); + const body = await resolveSkillBody(cwd, "shared", []); + expect(body).toContain("Project shared."); + expect(body).not.toContain("User shared."); + }); + } finally { + await rm(cwd, { recursive: true, force: true }); + await rm(home, { recursive: true, force: true }); + } + }); }); describe("skill resolution", () => { @@ -102,6 +193,25 @@ describe("skill resolution", () => { ).toBeUndefined(); }); + test("resolveSkillWithDir returns the body plus the winning skill directory", async () => { + const resolved = await resolveSkillWithDir( + fixtureCwd, + "scribe", + pluginDirs, + ); + expect(resolved).toBeDefined(); + expect(defined(resolved, "resolved skill").body).toContain("Scribe"); + expect(defined(resolved, "resolved skill").dir).toBe( + join(exampleAgentPlugin, "skills", "scribe"), + ); + }); + + test("resolveSkillWithDir returns undefined for an unknown skill", async () => { + expect( + await resolveSkillWithDir(fixtureCwd, "does-not-exist-xyz", pluginDirs), + ).toBeUndefined(); + }); + test("resolveSkillBody still loads disable-model-invocation skills by name", async () => { const plugin = await mkdtemp(join(tmpdir(), "skill-dmi-resolve-")); try { @@ -111,17 +221,77 @@ describe("skill resolution", () => { "---\nname: git-worktrees\nuser-invocable: false\ndisable-model-invocation: true\ndescription: bg\n---\nCreate worktree recipe.\n", "utf8", ); - expect(await discoverSkills(plugin, [plugin])).toEqual([]); - const body = await resolveSkillBody(plugin, "git-worktrees", [plugin]); - expect(body).toBeDefined(); - expect(body).toContain("Create worktree recipe."); - expect(defined(body, "skill body").startsWith("---")).toBe(false); + const emptyHome = await mkdtemp(join(tmpdir(), "skill-dmi-home-")); + try { + await withMockedHomedir(emptyHome, async () => { + expect(await discoverSkills(plugin, [plugin])).toEqual([]); + const body = await resolveSkillBody(plugin, "git-worktrees", [ + plugin, + ]); + expect(body).toBeDefined(); + expect(body).toContain("Create worktree recipe."); + expect(defined(body, "skill body").startsWith("---")).toBe(false); + }); + } finally { + await rm(emptyHome, { recursive: true, force: true }); + } } finally { await rm(plugin, { recursive: true, force: true }); } }); }); +describe("bare-name symlink escape", () => { + test("file-symlink SKILL.md pointing outside fails closed", async () => { + const cwd = await mkdtemp(join(tmpdir(), "skill-bare-filelink-cwd-")); + const outside = await mkdtemp(join(tmpdir(), "skill-bare-filelink-out-")); + const home = await mkdtemp(join(tmpdir(), "skill-bare-filelink-home-")); + try { + await mkdir(join(cwd, ".agents", "skills", "evil"), { recursive: true }); + await writeFile( + join(outside, "SKILL.md"), + "escaped outside body\n", + "utf8", + ); + await symlink( + join(outside, "SKILL.md"), + join(cwd, ".agents", "skills", "evil", "SKILL.md"), + ); + await withMockedHomedir(home, async () => { + expect(await resolveSkillBody(cwd, "evil", [])).toBeUndefined(); + expect(await resolveSkillWithDir(cwd, "evil", [])).toBeUndefined(); + }); + } finally { + await rm(cwd, { recursive: true, force: true }); + await rm(outside, { recursive: true, force: true }); + await rm(home, { recursive: true, force: true }); + } + }); + + test("dir-symlink skill dir pointing outside fails closed", async () => { + const cwd = await mkdtemp(join(tmpdir(), "skill-bare-dirlink-cwd-")); + const outside = await mkdtemp(join(tmpdir(), "skill-bare-dirlink-out-")); + const home = await mkdtemp(join(tmpdir(), "skill-bare-dirlink-home-")); + try { + await mkdir(join(cwd, ".agents", "skills"), { recursive: true }); + await writeFile( + join(outside, "SKILL.md"), + "escaped outside body\n", + "utf8", + ); + await symlink(outside, join(cwd, ".agents", "skills", "evildir"), "dir"); + await withMockedHomedir(home, async () => { + expect(await resolveSkillBody(cwd, "evildir", [])).toBeUndefined(); + expect(await resolveSkillWithDir(cwd, "evildir", [])).toBeUndefined(); + }); + } finally { + await rm(cwd, { recursive: true, force: true }); + await rm(outside, { recursive: true, force: true }); + await rm(home, { recursive: true, force: true }); + } + }); +}); + describe("path-like skill refs", () => { let pluginRoot: string; diff --git a/src/extensions/skills.ts b/src/extensions/skills.ts index d989d671a..596f94867 100644 --- a/src/extensions/skills.ts +++ b/src/extensions/skills.ts @@ -1,9 +1,11 @@ +import { homedir } from "node:os"; import { realpath } from "node:fs/promises"; import { readdir, readFile } from "node:fs/promises"; -import { basename, isAbsolute, join, resolve } from "node:path"; +import { basename, dirname, isAbsolute, join, resolve } from "node:path"; import { pathIsInsideOrEqual } from "../util/path-contain.js"; -const FALLBACK_SKILL_DIRS = [ +const SKILL_RELATIVE_DIRS = [ + ".corbits/skills", ".agents/skills", ".claude/skills", ".codex/skills", @@ -22,15 +24,15 @@ export interface ResolveSkillBodyOptions { */ pluginRoot?: string; /** - * Skip project-local `.agents/.claude/.codex/skills` fallbacks. Attached - * bundled product skills use this so a repo SKILL.md cannot - * become system-prompt constraints. + * Skip project-local and user-global skill dirs. Attached bundled product + * skills use this so a repo SKILL.md cannot become system-prompt constraints. */ pluginDirsOnly?: boolean; } -// Skill subfolders live under enabled plugin dirs first, then project-local dirs -// unless the caller opts out of the fallback. +// Skill subfolders live under enabled plugin dirs first, then project-local +// dirs, then user-global dirs, unless the caller opts out of those fallbacks. +// First-wins: plugin > project (.corbits, .agents, .claude, .codex) > user. function skillBaseDirs( cwd: string, pluginDirs: string[], @@ -38,7 +40,24 @@ function skillBaseDirs( ): string[] { const pluginBases = pluginDirs.map((dir) => join(dir, "skills")); if (!includeProjectFallback) return pluginBases; - return [...pluginBases, ...FALLBACK_SKILL_DIRS.map((rel) => join(cwd, rel))]; + const projectBases = SKILL_RELATIVE_DIRS.map((rel) => join(cwd, rel)); + const home = homedir(); + const userBases: string[] = []; + for (const rel of SKILL_RELATIVE_DIRS) { + const userPath = join(home, rel); + const projectPath = join(cwd, rel); + if (resolve(userPath) === resolve(projectPath)) continue; + userBases.push(userPath); + } + return [...pluginBases, ...projectBases, ...userBases]; +} + +// User-global skill base dirs (`~/.corbits/skills`, …). Project-local skill +// dirs resolve under the session cwd (already inside the workspace); these +// home-based dirs live outside it, so read-only file tools need them as +// trusted roots to open sibling files (see assembleSessionGate). +export function userSkillBaseDirs(home: string = homedir()): string[] { + return SKILL_RELATIVE_DIRS.map((rel) => join(home, rel)); } function parseSkillRef(ref: string): string { @@ -108,16 +127,26 @@ async function bodyFromSkillPath(path: string): Promise { return body.length > 0 ? body : undefined; } +/** + * A resolved skill: the SKILL.md body (frontmatter stripped) plus the skill + * directory that won, so callers can point the model at sibling files + * (e.g. brand-guidelines.md) living next to SKILL.md. + */ +export interface ResolvedSkill { + body: string; + dir: string; +} + /** * Resolve a path-like skill ref against `pluginRoot`. Absolute refs and * escapes outside the root are rejected. Accepts a SKILL.md file path or a * directory that contains SKILL.md. When the candidate exists, both sides are * realpath'd so a symlink under the root cannot escape to outside content. */ -async function resolvePathLikeSkillBody( +async function resolvePathLikeSkill( pluginRoot: string, ref: string, -): Promise { +): Promise { if (isAbsolute(ref)) return undefined; const root = resolve(pluginRoot); const resolved = resolve(root, ref); @@ -135,7 +164,9 @@ async function resolvePathLikeSkillBody( realpath(root), ]); if (!pathIsInsideOrEqual(realSkill, realRoot)) return undefined; - return bodyFromSkillPath(realSkill); + const body = await bodyFromSkillPath(realSkill); + if (body === undefined) return undefined; + return { body, dir: dirname(realSkill) }; } catch { // Missing path (or unreadable root) → not a resolvable skill. return undefined; @@ -146,34 +177,63 @@ async function resolvePathLikeSkillBody( // frontmatter is stripped, leaving the instructions to inject into context. // // Bare names search `skillBaseDirs` (plugin dirs then, unless pluginDirsOnly, -// project-local fallbacks). Path-like refs (`./skills/style`, `skills/foo`) -// resolve only under `options.pluginRoot` with containment checks; absolute -// and escape paths fail. -export async function resolveSkillBody( +// project-local then user-global fallbacks). Path-like refs (`./skills/style`, +// `skills/foo`) resolve only under `options.pluginRoot` with containment +// checks; absolute and escape paths fail. +export async function resolveSkillWithDir( cwd: string, ref: string, pluginDirs: string[] = [], options?: ResolveSkillBodyOptions, -): Promise { +): Promise { const name = parseSkillRef(ref); // Bare `.` / `..` are not skill names and must not fall through to directory search. if (name === "." || name === "..") return undefined; if (isPathLikeSkillRef(name)) { const pluginRoot = options?.pluginRoot; if (pluginRoot === undefined) return undefined; - return resolvePathLikeSkillBody(pluginRoot, name); + return resolvePathLikeSkill(pluginRoot, name); } for (const base of skillBaseDirs( cwd, pluginDirs, options?.pluginDirsOnly !== true, )) { - const body = await bodyFromSkillPath(join(base, name, "SKILL.md")); - if (body !== undefined) return body; + const dir = join(base, name); + const skillMd = join(dir, "SKILL.md"); + // Symlink escape bar (mirrors resolvePathLikeSkill): realpath both sides + // so a skill dir or SKILL.md symlinked outside the winning base fails + // closed instead of returning outside content. + let realSkill: string; + let realBase: string; + try { + [realSkill, realBase] = await Promise.all([ + realpath(skillMd), + realpath(base), + ]); + } catch { + // Missing base or skill → not resolvable here. + continue; + } + if (!pathIsInsideOrEqual(realSkill, realBase)) continue; + const body = await bodyFromSkillPath(realSkill); + // Return the lexical dir (unchanged public semantics): containment of the + // realpath'd SKILL.md in the realpath'd base already failed closed above. + if (body !== undefined) return { body, dir }; } return undefined; } +export async function resolveSkillBody( + cwd: string, + ref: string, + pluginDirs: string[] = [], + options?: ResolveSkillBodyOptions, +): Promise { + const resolved = await resolveSkillWithDir(cwd, ref, pluginDirs, options); + return resolved?.body; +} + // Discover every available skill (name + one-line description). Deduped by name: // the first base dir that provides a skill wins, so a higher-precedence dir // shadows a lower one. Descriptions feed skill_search and the slash picker; the diff --git a/src/plugins/agent-plugins.test.ts b/src/plugins/agent-plugins.test.ts index a95ba53fc..ae8b6a956 100644 --- a/src/plugins/agent-plugins.test.ts +++ b/src/plugins/agent-plugins.test.ts @@ -146,33 +146,46 @@ describe("resolveAgentPluginProfiles", () => { ).toEqual([]); }); - test("skips profiles whose id collides with a closed DIRECTOR_IDS entry", async () => { + test("replaces profiles whose id collides with a closed DIRECTOR_IDS entry", async () => { const warnings: string[] = []; const { mod, config } = agentModule("p1", [ validProfile, { id: "explorer", - description: "Collides with closed director", - systemPromptRole: "Should be skipped.", + description: "Overlay closed director", + systemPromptRole: "Plugin explorer prompt.", }, { - id: "coder", - description: "Also reserved", - systemPromptRole: "Should be skipped.", + id: "designer", + description: "Also overlay", + systemPromptRole: "Plugin designer prompt.", }, ]); const profiles = await resolveAgentPluginProfiles([mod], config, (msg) => warnings.push(msg), ); - expect(profiles.map((p) => p.id)).toEqual(["scout"]); + expect(profiles.map((p) => p.id).sort()).toEqual([ + "designer", + "explorer", + "scout", + ]); + expect( + defined(profiles.find((p) => p.id === "designer")).systemPromptRole, + ).toBe("Plugin designer prompt."); expect( warnings.some( - (w) => w.includes('agent "explorer"') && w.includes("reserved"), + (w) => + w.includes('plugin "p1"') && + w.includes("agent explorer") && + w.includes("replaces closed director"), ), ).toBe(true); expect( warnings.some( - (w) => w.includes('agent "coder"') && w.includes("reserved"), + (w) => + w.includes('plugin "p1"') && + w.includes("agent designer") && + w.includes("replaces closed director"), ), ).toBe(true); }); diff --git a/src/plugins/agent-plugins.ts b/src/plugins/agent-plugins.ts index bde722ea1..783141126 100644 --- a/src/plugins/agent-plugins.ts +++ b/src/plugins/agent-plugins.ts @@ -44,9 +44,9 @@ function resolveAgentProfileWarningHandler( // explicit enabled+consented in settings even for repo plugins, because a // tool plugin runs in-process code rather than declaring configuration data. // -// Closed DIRECTOR_IDS are reserved (CL-7015): a plugin profile whose id -// collides with a shipped director is skipped with a warning — plugins cannot -// override or alias the closed fleet. +// Closed director ids are overridable (CL-9917): a trusted/enabled plugin +// profile whose id collides with a shipped director replaces the closed +// package at spawn. Last enabled plugin wins via later mergeProfileInto. // // Warnings fire whenever a profile is rejected so JS-plugin authors get the // same feedback loop data-only plugin authors already enjoy. Pass `diagnostics` @@ -80,9 +80,8 @@ export async function resolveAgentPluginProfiles( const profile = { ...(result as AgentProfile) }; if (isDirectorId(profile.id)) { onWarning( - `plugin "${mod.manifest.id}" agent "${profile.id}" skipped: id is reserved for a closed director`, + `plugin "${mod.manifest.id}" agent ${profile.id} replaces closed director`, ); - continue; } // Resolve systemPromptPath relative to the plugin directory. The file // content becomes systemPromptRole; an explicit systemPromptRole wins. diff --git a/src/session/assemble-runtime.ts b/src/session/assemble-runtime.ts index 2a4be3a1d..887dacbd1 100644 --- a/src/session/assemble-runtime.ts +++ b/src/session/assemble-runtime.ts @@ -107,6 +107,7 @@ import { } from "../permission/gate.js"; import type { Approval, RequestApproval } from "../permission/types.js"; import { createWorktreeRootsProvider } from "../permission/worktree-roots.js"; +import { userSkillBaseDirs } from "../extensions/skills.js"; import { userPluginsRoot } from "../plugins/uninstall.js"; import { createApprovalLog } from "../permission/approval-log.js"; import { sessionDir } from "./index.js"; @@ -312,7 +313,11 @@ export async function assembleSessionGate( telemetry: args.telemetry, cwd: args.cwd, rootsProvider: createWorktreeRootsProvider(args.cwd), - trustedPluginRoots: () => [userPluginsRoot()], + // User-global skill dirs live outside the workspace, so sibling reads + // (read_file/grep/search_files/list_dir of files next to a loaded + // SKILL.md) need them as trusted roots. Trusted roots are read-only by + // construction — writes/deletes under them stay hard escape denies. + trustedPluginRoots: () => [userPluginsRoot(), ...userSkillBaseDirs()], providerName: args.providerName, model: args.model, requestApproval: args.requestApproval, diff --git a/src/subagent/agent-fleet.test.ts b/src/subagent/agent-fleet.test.ts index 20fba96e7..ade3e8792 100644 --- a/src/subagent/agent-fleet.test.ts +++ b/src/subagent/agent-fleet.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { mkdtemp, readFile } from "node:fs/promises"; +import { mkdir, mkdtemp, readFile, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -421,6 +421,135 @@ describe("spawn_agent", () => { expect(captured?.tier).toBe("leaf"); expect(captured?.orchestrator).toBeUndefined(); }); + + test("plugin profile with closed director id replaces the closed package prompt", async () => { + let captured: RunSubAgentParams | undefined; + const deps = createFleetDeps( + async (params) => { + captured = params; + return { report: "done" }; + }, + { + profiles: [ + { + id: "designer", + source: "plugin:p1", + systemPromptRole: "You are the plugin designer.", + optionalSkills: ["better-ui"], + }, + ], + }, + ); + const spawn = createSpawnAgentTool(deps); + + const result = await callFleetTool(spawn, { + description: "design job", + prompt: "style this", + agent: "designer", + }); + + expect(result.status).toBe("running"); + expect(captured?.directorId).toBe("designer"); + expect(captured?.systemPromptRole).toBe("You are the plugin designer."); + expect(captured?.allowedSkillNames).toEqual(["better-ui"]); + }); + + test("spawn skills[] unions with the package allowlist; omitted stays fail-closed", async () => { + const fixture = await mkdtemp(join(tmpdir(), "corbits-spawn-skills-")); + const skillDir = join(fixture, ".agents", "skills", "custom"); + await mkdir(skillDir, { recursive: true }); + await writeFile( + join(skillDir, "SKILL.md"), + "---\nname: custom\ndescription: Custom lane.\n---\nCustom body.\n", + ); + + const captured: RunSubAgentParams[] = []; + const deps = createFleetDeps( + async (params) => { + captured.push(params); + return { report: "done" }; + }, + { + profiles: [ + { + id: "bare-worker", + systemPromptRole: "You are a bare worker with no package skills.", + }, + ], + }, + ); + const spawn = createSpawnAgentTool(deps); + + // Package has no skill fields: spawn skills alone open the allowlist, + // and blank entries are ignored, never an error. + const withSkills = await callFleetTool(spawn, { + description: "skilled job", + prompt: "do it", + agent: "bare-worker", + skills: ["custom", " ", ""], + }); + expect(withSkills.status).toBe("running"); + expect(captured[0]?.allowedSkillNames).toEqual(["custom"]); + + // The dispatched allowlist is what gates use_skill: the spawned worker + // loads the custom skill through it. + const { createUseSkillTool } = await import("../agent/use-skill.js"); + const callUseSkill = ( + tool: ReturnType, + args: Record, + ): Promise => { + if (tool.kind !== "string") throw new Error("expected string tool"); + return tool.handler(args, new AbortController().signal); + }; + const skilled = createUseSkillTool( + fixture, + [], + undefined, + captured[0]?.allowedSkillNames, + ); + expect(await callUseSkill(skilled, { name: "custom" })).toContain( + "Custom body.", + ); + + // Without the spawn handoff the same package stays fail-closed. + const withoutSkills = await callFleetTool(spawn, { + description: "bare job", + prompt: "do it", + agent: "bare-worker", + }); + expect(withoutSkills.status).toBe("running"); + expect(captured[1]?.allowedSkillNames).toEqual([]); + const bare = createUseSkillTool( + fixture, + [], + undefined, + captured[1]?.allowedSkillNames, + ); + expect(await callUseSkill(bare, { name: "custom" })).toBe( + 'No skill named "custom" is available.', + ); + }); + + test("spawn skills[] unions with a closed package allowlist", async () => { + const captured: RunSubAgentParams[] = []; + const deps = createFleetDeps(async (params) => { + captured.push(params); + return { report: "done" }; + }); + const spawn = createSpawnAgentTool(deps); + + // coder declares optionalSkills ["typescript"]: the spawn list unions, + // it does not replace. + const result = await callFleetTool(spawn, { + description: "coded job", + prompt: "implement it", + agent: "coder", + success_criteria: ["it ships"], + skills: ["custom"], + }); + expect(result.status).toBe("running"); + expect(captured[0]?.allowedSkillNames).toEqual(["typescript", "custom"]); + }); }); describe("spawn_agent + wait_agents", () => { diff --git a/src/subagent/agent-fleet.ts b/src/subagent/agent-fleet.ts index f1701fb33..7ecfa59b6 100644 --- a/src/subagent/agent-fleet.ts +++ b/src/subagent/agent-fleet.ts @@ -53,11 +53,13 @@ import { isDirectorId, packageToCapabilities, resolveDirector, + DIRECTOR_REGISTRY, } from "../agent/directors/registry.js"; import { defaultEffortForDirector, formatDirectorSystemPrompt, packageAllowedSkillNames, + skillNamesFromFields, } from "../agent/directors/identity.js"; import type { Settings } from "../config/settings.js"; import { resolveInferenceWithPolicy } from "../config/settings.js"; @@ -556,6 +558,7 @@ const SpawnAgentArgs = type({ "do_not?": "string[]", "report_focus?": "string", "requires_tools?": "string[]", + "skills?": "string[]", }); export const spawnAgentToolDefinition: ToolDefinition = { @@ -608,6 +611,12 @@ export const spawnAgentToolDefinition: ToolDefinition = { description: "Hard tool requirements (canonical names). Preflight fails closed when missing.", }, + skills: { + type: "array", + items: { type: "string" }, + description: + "Extra skill names for this worker only; unioned with package allowlist.", + }, }, required: ["description", "prompt"], }, @@ -752,6 +761,8 @@ interface ResolvedAgentDispatch { orchestratorTier?: DirectorPackage["tier"]; nestedSpawnAllowlist?: readonly string[]; effortPin?: ReasoningEffort; + attachedSkills?: readonly string[]; + optionalSkills?: readonly string[]; } function resolveAgentDispatch(input: { @@ -778,12 +789,19 @@ function resolveAgentDispatch(input: { applyResolvedProvider, } = input; if (agentId !== undefined && agentId.length > 0) { - if (isDirectorId(agentId)) { + const overlayProfile = profiles?.find((p) => p.id === agentId); + // Plugin/local overlay (source set) replaces the closed package. Default + // directorProfiles() entries omit source, so they stay on the closed path. + const useClosedDirector = + isDirectorId(agentId) && + (overlayProfile === undefined || overlayProfile.source === undefined); + + if (useClosedDirector) { const resolved = resolveDirector({ agentId }); if (!resolved.ok) return { error: `Error: ${resolved.error} ${resolved.hint}` }; const pkg = resolved.package; - const profile = profiles?.find((p) => p.id === agentId); + const profile = overlayProfile; let effortPin: ReasoningEffort | undefined; if (profile?.inference !== undefined && settings !== undefined) { const outcome = resolveInferenceWithPolicy(profile.inference, settings); @@ -826,7 +844,7 @@ function resolveAgentDispatch(input: { error: `Error: agent "${agentId}" requested but no agent profiles are loaded. Omit agent to use intent=, or ensure profiles are available.`, }; } - const profile = profiles.find((p) => p.id === agentId); + const profile = overlayProfile; if (profile === undefined) { const known = profiles.map((p) => p.id).sort(); const hint = @@ -868,6 +886,15 @@ function resolveAgentDispatch(input: { : {}), orchestrator: false, ...(effortPin !== undefined ? { effortPin } : {}), + ...(isDirectorId(agentId) + ? { roleDefault: defaultEffortForDirector(DIRECTOR_REGISTRY[agentId]) } + : {}), + ...(profile.attachedSkills !== undefined + ? { attachedSkills: profile.attachedSkills } + : {}), + ...(profile.optionalSkills !== undefined + ? { optionalSkills: profile.optionalSkills } + : {}), }; } @@ -993,6 +1020,7 @@ export function createSpawnAgentTool(deps: AgentFleetDeps): AgentTool { do_not: rawDoNot, report_focus: rawReportFocus, requires_tools: rawRequiresTools, + skills: rawSkills, } = parsed; const description = rawDesc.trim(); const prompt = rawPrompt.trim(); @@ -1012,6 +1040,10 @@ export function createSpawnAgentTool(deps: AgentFleetDeps): AgentTool { const doNot = rawDoNot?.map((d) => d.trim()).filter((d) => d.length > 0) ?? []; const reportFocus = rawReportFocus?.trim(); + // Per-spawn skill handoff: extra skill names for this worker only. + // Arktype guarantees strings; blank entries are ignored, never an error. + const spawnSkills = + rawSkills?.map((s) => s.trim()).filter((s) => s.length > 0) ?? []; let provider: SubAgentProvider = resolveDep(deps.provider); const parentEffort = provider.reasoningEffort; @@ -1468,7 +1500,21 @@ export function createSpawnAgentTool(deps: AgentFleetDeps): AgentTool { modelRole: laneModelRole, }); - const allowedSkillNames = packageAllowedSkillNames(resolved.pkg); + const baseAllowedSkillNames = + packageAllowedSkillNames(resolved.pkg) ?? + skillNamesFromFields( + resolved.attachedSkills, + resolved.optionalSkills, + ); + // Per-spawn skill handoff: `skills` names are for this worker + // only, unioned with the package allowlist (order-preserving, + // first-wins). Empty spawn list keeps the package scope as-is. + const allowedSkillNames = + spawnSkills.length > 0 + ? skillNamesFromFields(baseAllowedSkillNames, spawnSkills) + : baseAllowedSkillNames; + const attachedSkills = + resolved.pkg?.attachedSkills ?? resolved.attachedSkills; const params: RunSubAgentParams = { // Name the trace directory after the session-store id so the // descendant-scoping check behind read_agent_trace can resolve this @@ -1527,10 +1573,9 @@ export function createSpawnAgentTool(deps: AgentFleetDeps): AgentTool { ? { capabilities: resolved.capabilities } : {}), ...(requiresTools !== undefined ? { requiresTools } : {}), - ...(allowedSkillNames !== undefined ? { allowedSkillNames } : {}), - ...(resolved.pkg?.attachedSkills !== undefined && - resolved.pkg.attachedSkills.length > 0 - ? { attachedSkills: resolved.pkg.attachedSkills } + allowedSkillNames, + ...(attachedSkills !== undefined && attachedSkills.length > 0 + ? { attachedSkills } : {}), ...(deps.skillDirs !== undefined ? { skillDirs: deps.skillDirs } diff --git a/src/subagent/run-skill-scope.test.ts b/src/subagent/run-skill-scope.test.ts index 997513487..11ca03172 100644 --- a/src/subagent/run-skill-scope.test.ts +++ b/src/subagent/run-skill-scope.test.ts @@ -140,10 +140,9 @@ describe("runSubAgent worker skill mounts (CL-7668)", () => { const useSkillArgs = captured.useSkillArgs[0]; expect(searchArgs).toBeDefined(); expect(searchArgs?.allowedNames).toEqual(["style"]); - expect(searchArgs?.skills.map((s) => s.name).sort()).toEqual([ - "off-lane", - "style", - ]); + const discovered = searchArgs?.skills.map((s) => s.name) ?? []; + expect(discovered).toContain("off-lane"); + expect(discovered).toContain("style"); expect(useSkillArgs?.[0]).toBe(cwd); expect(useSkillArgs?.[1]).toEqual([]); expect(useSkillArgs?.[3]).toEqual(["style"]); @@ -350,4 +349,34 @@ describe("runSubAgent worker skill mounts (CL-7668)", () => { ); expect(joined).not.toContain("### philosophy"); }, 15_000); + + test("empty allowedSkillNames fail-closes: use_skill cannot load typescript", async () => { + const cwd = await tmpSubAgentCwd("cl9917-skill-fail-closed-"); + await writeSkill( + cwd, + "typescript", + "TypeScript conventions.", + "Prefer explicit types.", + ); + + const { captured, withCapturedMounts } = captureSkillMounts(); + await runWithFailingInference((baseURL) => + withCapturedMounts(() => + probeRun(cwd, baseURL, { allowedSkillNames: [] }), + ), + ); + + expect(captured.searchArgs[0]?.allowedNames).toEqual([]); + expect(captured.useSkillArgs[0]?.[3]).toEqual([]); + const refused = await captured.useSkillTool?.handler( + { name: "typescript" }, + new AbortController().signal, + ); + expect(refused).toBe('No skill named "typescript" is available.'); + const search = await captured.searchTool?.handler( + { query: "typescript" }, + new AbortController().signal, + ); + expect(search).toContain("No skills matched"); + }, 15_000); }); diff --git a/src/subagent/types.ts b/src/subagent/types.ts index ed5a585bc..bdb643998 100644 --- a/src/subagent/types.ts +++ b/src/subagent/types.ts @@ -157,11 +157,11 @@ export type RunSubAgentParams = { requiresTools?: readonly string[]; /** * Skill allowlist for the worker's skill_search + use_skill mounts, - * resolved by the caller (agent-fleet.ts) as the union of - * DirectorPackage.attachedSkills and optionalSkills. When set, both tools - * only see these names (the allowlist cannot widen: unknown names refuse). - * When unset (non-director plugin profiles), the worker sees every - * discovered skill. + * resolved by the caller (agent-fleet.ts) as the union of attachedSkills + * and optionalSkills on the director package or plugin profile. When set, + * both tools only see these names (the allowlist cannot widen: unknown + * names refuse). Empty means match nothing. Unset (legacy callers) still + * sees every discovered skill. */ allowedSkillNames?: readonly string[]; /**