commit c6d7d538f1c0f0a7e04ad429835e2f0aa5ee39b6 Author: Jason Woltje Date: Mon Oct 19 15:41:25 2026 -0500 roles, harness, cli: a managed session gets its role's skills as prompt text on both harnesses (row 64, #1557) Author and gate owner: Sage (mos-0855). Role files take an optional skills list. The bundle reads each named skill from /skills-enabled, copies it into the bundle and renders every file into prompt.md between skill-file markers, so the prompt text matches the bundle copy byte for byte. The role's skills together are capped at SKILL_BYTES (64 KiB): the claude adapter passes prompt.md as one argument, and Linux caps one argument at 128 KiB. Symlinks are refused, not followed. The launcher checks the list before a run exists: skill-not-enabled names the activate command, anything else refuses skill-refused. Pi keeps --no-skills; Claude Code keeps --disable-slash-commands and --restricted. The two harness modules and MOSAIC_SKILLS are unchanged. First lists: coder, reviewer, cto and pm, ms-unslop on each. diff --git a/adapters/README.md b/adapters/README.md index ab5d1793d..7636fd8c9 100644 --- a/adapters/README.md +++ b/adapters/README.md @@ -82,3 +82,27 @@ session's workspace and its own process group. Same contract, plus: The layers each adapter relies on, and what they don't cover, are in `packages/harness/README.md`. + +### A role's skills (row 64) + +A managed session's skills reach both adapters as text in the bundle's +`prompt.md`, which they pass with `--system-prompt`. Neither adapter loads a +skill of its own: `pi` keeps `--no-skills`, since role sessions never get +`MOSAIC_SKILLS`, and `claude` keeps `--disable-slash-commands` and +`--restricted`. Sage chose this on 2026-10-19 from a probe, recorded in +`agents/sage/work/queue-64/probe.md`: + +- Pi's `--skill` puts only a skill's name and description in the prompt, + and the model has to `read` the SKILL.md itself. The gate refuses every + file-tool path outside the workspace, so the session couldn't open its + own skills. Opening the gate to the bundle would change a security + boundary. +- Claude Code loads a bundle-local `--plugin-dir` skill under `--restricted` + only with its `Skill` tool on, and then it also offers its own bundled + skills. Which ones it offers changes with the Claude Code version, and the + `Skill` tool would pass through the gate hook. + +The cost is prompt size: a role's skill text is in every turn, where the +other routes load a skill when it's used. The bundle caps a role's skills at +64 KiB together, since `claude` passes `prompt.md` as one argument and Linux +caps one argument at 128 KiB. diff --git a/adapters/claude/adapter.sh b/adapters/claude/adapter.sh index ccf48c014..4f367b16e 100644 --- a/adapters/claude/adapter.sh +++ b/adapters/claude/adapter.sh @@ -72,7 +72,8 @@ fi # --settings the gate hook (S0 lines 1-4) # --strict-mcp-config only the bundle's MCP server, which serves the # --mcp-config typed bus tools -# --disable-slash-commands no skills (the bundle lists none) +# --disable-slash-commands no skill discovery; the role's skills are +# text in the system prompt (row 64) ALLOWED=mcp__mosaic [ -z "${MOSAIC_TOOLS:-}" ] || ALLOWED="$MOSAIC_TOOLS,mcp__mosaic" PROMPT_CONTENT="$(cat "$MOSAIC_SYSTEM_PROMPT_FILE")" diff --git a/adapters/pi/adapter.sh b/adapters/pi/adapter.sh index 486c1aba1..65db0a0c3 100644 --- a/adapters/pi/adapter.sh +++ b/adapters/pi/adapter.sh @@ -49,6 +49,9 @@ TOOLS_FLAG="--no-tools" # Skills (M17): explicitly provided skill dirs replace discovery. When none # are provided the agent runs with --no-skills (nothing ambient to find). +# Role sessions never get MOSAIC_SKILLS (ENV_ALLOW in packages/seat/src/ +# session.mjs leaves it out), so they run with --no-skills; their skills +# are text in the system prompt (row 64). SKILLS_FLAG="--no-skills" if [ -n "${MOSAIC_SKILLS:-}" ]; then SKILLS_FLAG="" diff --git a/packages/business/src/resolve.mjs b/packages/business/src/resolve.mjs index 499010c10..b6c071bfa 100644 --- a/packages/business/src/resolve.mjs +++ b/packages/business/src/resolve.mjs @@ -92,6 +92,8 @@ export function resolveInstance({ system, business, project = null, instance }) launch, // REQ-HARN-2: the search-source plug-in the organization file switches on, by name. webSource: business.plugins?.["search-source"] ?? null, + // The role file's skill names. The bundle resolves each against /skills-enabled. + skills: [...definition.skills], }; return deepFreeze({ ...resolved, digest: sha256(canonicalJson(resolved)) }); } diff --git a/packages/business/src/role.mjs b/packages/business/src/role.mjs index b990e8e12..b3c541d41 100644 --- a/packages/business/src/role.mjs +++ b/packages/business/src/role.mjs @@ -17,7 +17,8 @@ import { const CONTRACT_NAME = /^[a-z0-9][a-z0-9._-]{0,60}\.md$/; const V1_KEYS = ["roleVersion", "name", "tools", "network"]; const V2_REQUIRED = ["roleVersion", "name", "title", "contract", "tools", "network", "authority", "credentials"]; -const V2_KEYS = [...V2_REQUIRED, "messaging"]; +const V2_KEYS = [...V2_REQUIRED, "messaging", "skills"]; +const SKILL_NAME = /^[A-Za-z0-9._-]+$/; function checkTools(tools) { if (!Array.isArray(tools) || tools.length === 0) refuse('role "tools" must be a non-empty array of tool names'); @@ -64,6 +65,16 @@ function checkMessaging(messaging) { return lists; } +// Optional: the skills a session of this role gets, by their directory name under +// /skills-enabled. "." and ".." match the pattern but name no skill directory. +function checkSkills(skills) { + return requireDistinctList(skills, "role skills", (name) => { + if (typeof name !== "string" || !SKILL_NAME.test(name) || name === "." || name === "..") { + refuse(`role skill must be a skill directory name matching ${SKILL_NAME} (got ${JSON.stringify(name)})`); + } + }); +} + function checkGiteaScopes(scopes) { const list = requireDistinctList(scopes, "gitea scopes", (scope) => { const m = typeof scope === "string" ? /^(read|write):([a-z]+)$/.exec(scope) : null; @@ -123,7 +134,7 @@ function checkContract(contract, file) { // Validate a parsed role document read from `file`. Returns a frozen role: // { roleVersion, name, title, contract, contractPath, tools, network, // authority: { withinRole, crossRole }, credentials: [{ service, scopes }], -// messaging: { sendTo, receiveFrom } | null }. +// messaging: { sendTo, receiveFrom } | null, skills: [name] }. export function validateRoleDocument(document, file) { requireObject(document, "role"); if (document.roleVersion !== 1 && document.roleVersion !== 2) refuse('role "roleVersion" must be 1 or 2'); @@ -136,7 +147,7 @@ export function validateRoleDocument(document, file) { const network = document.network === undefined ? "none" : checkNetwork(document.network); return deepFreeze({ roleVersion: 1, name: document.name, title: null, contract: null, contractPath: null, tools, network, - authority: { withinRole: [], crossRole: [] }, credentials: [], messaging: null, + authority: { withinRole: [], crossRole: [] }, credentials: [], messaging: null, skills: [], }); } for (const key of V2_REQUIRED) { @@ -153,6 +164,7 @@ export function validateRoleDocument(document, file) { authority: checkAuthority(document.authority), credentials: checkCredentials(document.credentials), messaging: document.messaging === undefined ? null : checkMessaging(document.messaging), + skills: document.skills === undefined ? [] : checkSkills(document.skills), }); } diff --git a/packages/business/tests/resolve.test.mjs b/packages/business/tests/resolve.test.mjs index 36fa9ee36..5ce7298ca 100644 --- a/packages/business/tests/resolve.test.mjs +++ b/packages/business/tests/resolve.test.mjs @@ -154,7 +154,7 @@ test("the record carries what the broker and launcher need", () => { const s = setup(); const pm = resolve(s, "pm"); assert.deepEqual(Object.keys(pm).sort(), [ - "business", "contract", "credentials", "definition", "digest", "holder", "instance", "launch", "limits", "project", "provenance", "tracker", "vars", "webSource", + "business", "contract", "credentials", "definition", "digest", "holder", "instance", "launch", "limits", "project", "provenance", "skills", "tracker", "vars", "webSource", ]); assert.equal(pm.contract, join(REPO_ROLES, "pm.md")); assert.equal(pm.holder, "sage"); diff --git a/packages/business/tests/role.test.mjs b/packages/business/tests/role.test.mjs index 48917bb68..1ff23dcab 100644 --- a/packages/business/tests/role.test.mjs +++ b/packages/business/tests/role.test.mjs @@ -161,6 +161,21 @@ test("messaging: optional, role definitions and human, never the tester", () => refuses(() => loadRoleFile(file), /unsupported role key: "messaging"/); }); +test("row 64 skills: optional, distinct skill directory names, none on version 1", () => { + assert.deepEqual(check({}).skills, []); + const role = check({ skills: ["ms-unslop", "ms_sdlc.v2"] }); + assert.deepEqual(role.skills, ["ms-unslop", "ms_sdlc.v2"]); + assert.ok(Object.isFrozen(role.skills)); + refuses(() => check({ skills: "ms-unslop" }), /role skills must be an array/); + refuses(() => check({ skills: ["ms-unslop", "ms-unslop"] }), /duplicate entry in role skills: "ms-unslop"/); + for (const bad of [".", "..", "", "a/b", "../ms-unslop", "ms unslop", 7, null]) { + refuses(() => check({ skills: [bad] }), /role skill must be a skill directory name/); + } + const file = writeJson(join(tmp(), "v1s.json"), { roleVersion: 1, name: "v1s", tools: ["read"], skills: ["ms-unslop"] }); + refuses(() => loadRoleFile(file), /unsupported role key: "skills"/); + assert.deepEqual(validateRoleDocument({ roleVersion: 1, name: "v1s", tools: ["read"] }, file).skills, []); +}); + test("credentials: Gitea scopes", () => { const gitea = (scopes) => check({ credentials: [{ service: "gitea", scopes }] }); assert.deepEqual(gitea(["write:repository", "read:user"]).credentials[0].scopes, ["write:repository", "read:user"]); diff --git a/packages/cli/src/launcher.mjs b/packages/cli/src/launcher.mjs index 6cb9c4f00..fb0f9f647 100644 --- a/packages/cli/src/launcher.mjs +++ b/packages/cli/src/launcher.mjs @@ -101,7 +101,7 @@ import { chmodSync, existsSync, lstatSync, mkdirSync, readFileSync, rmSync } fro import { join } from "node:path"; import { configDir, loadBusiness, resolveInstance, systemVars } from "../../business/src/index.mjs"; import { limitHit } from "../../business/src/launch.mjs"; -import { BundleError, buildBundle, hasFallback, sessionModel } from "../../harness/src/bundle.mjs"; +import { BundleError, buildBundle, hasFallback, readSkills, sessionModel } from "../../harness/src/bundle.mjs"; import { createLoader } from "../../plugins/src/loader.mjs"; import { appendLaunchLog, @@ -173,6 +173,8 @@ export function createLauncher({ log = (l) => process.stderr.write(`mosaic-bus: ${l}\n`), }) { const dataRoot = system.dataRoot; + // Row 64 (#1557): where a role's skills are read from; scripts/skill.sh activate fills it. + const skillsDir = join(dataRoot, "skills-enabled"); let b = loadBusiness(host.business, { dir: configDir(env), rolesDir: rolesDir(env) }); const vars = systemVars(system); @@ -247,6 +249,13 @@ export function createLauncher({ const primary = routeOf("primary"); const fallback = hasFallback(resolved) ? routeOf("fallback") : null; if (!resolved.contract) throw new Refusal("no-role-contract", `role ${resolved.definition} has no contract file`); + // Row 64 (#1557): every skill on the role's list is enabled and readable before a run exists. + try { + readSkills(resolved.skills, skillsDir); + } catch (e) { + if (e instanceof BundleError) throw new Refusal(e.code === "skill-not-enabled" ? "skill-not-enabled" : "skill-refused", e.message); + throw e; + } // Both of a launch's harnesses must be switched on, whichever it takes. const on = Array.isArray(b.plugins?.harness) ? b.plugins.harness : []; for (const m of [primary, fallback].filter(Boolean)) { @@ -373,6 +382,7 @@ export function createLauncher({ routeReason, harnessVersion: version, land, + skillsDir, }); const { manifest, ...files } = bundle.files; const adapter = call(model.harness, "adapter", () => harness.adapter()); diff --git a/packages/cli/tests/channel-fixture.mjs b/packages/cli/tests/channel-fixture.mjs index b3a04d9b3..1b7211209 100644 --- a/packages/cli/tests/channel-fixture.mjs +++ b/packages/cli/tests/channel-fixture.mjs @@ -1,7 +1,7 @@ // Private fixture runtime only; no live host, unit, store or organization file. import { spawn } from "node:child_process"; import { once } from "node:events"; -import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { fileURLToPath } from "node:url"; @@ -33,7 +33,6 @@ export async function provedRuntime(t) { } finally { rmSync(root, { recursive: true, force: true }); } }); const f = fixture(root, "demo", doc => ({ ...doc, plugins: { channel: [] } })); - mkdirSync(f.dataRoot, { mode: 0o700 }); const url = rel => new URL(rel, import.meta.url).href; const humanURL = url("../../bus/src/human.mjs"); const script = join(root, "fixture-runtime.mjs"); diff --git a/packages/cli/tests/git-socket.test.mjs b/packages/cli/tests/git-socket.test.mjs index 5284189c9..e0d76af1b 100644 --- a/packages/cli/tests/git-socket.test.mjs +++ b/packages/cli/tests/git-socket.test.mjs @@ -73,7 +73,6 @@ async function setup(t, { wrap = (h) => h, ...git } = {}) { } }); const f = fixture(root, "acme"); - mkdirSync(f.dataRoot, { mode: 0o700 }); const project = f.doc.projects.stack.root; gitIn(project, "init", "-q", "-b", "main"); const main = commit(project, "one"); diff --git a/packages/cli/tests/helpers.mjs b/packages/cli/tests/helpers.mjs index b03045c1d..2ee9580bf 100644 --- a/packages/cli/tests/helpers.mjs +++ b/packages/cli/tests/helpers.mjs @@ -2,7 +2,7 @@ // points MOSAIC_CONFIG there; nothing reads the real ~/.config, a real // token, a real binding, or the real human transport. -import { chmodSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { chmodSync, cpSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { PassThrough } from "node:stream"; @@ -10,6 +10,7 @@ import { pathToFileURL } from "node:url"; import { BusError, Broker } from "../../bus/src/broker.mjs"; import { Store } from "../../bus/src/store.mjs"; import { businessDoc, rolesCopy, systemConfig, writeJson } from "../../business/tests/helpers.mjs"; +import { REPO_SKILLS } from "../../harness/tests/helpers.mjs"; import { tree } from "../../plugins/tests/helpers.mjs"; import { AGENT_MARKERS } from "../src/transport.mjs"; @@ -31,7 +32,9 @@ export function humanEnv(extra = {}) { // A config directory with the system config, the roles and business `id`. Row 65: both harnesses // are switched on, since a launch on one that isn't refuses harness-off, and every instance names a // harness, model and thinking level, as the live file does, since a launch missing one refuses -// no-route. The instance's own vars win, and edit can change any of it. +// no-route. The instance's own vars win, and edit can change any of it. Row 64: the data root's +// skills-enabled holds the repository's skills, since a launch whose role names one that isn't +// there refuses skill-not-enabled. The fixture makes the data root (0700) to hold them. export function fixture(root, id = "acme", edit = (doc) => doc) { const config = systemConfig(root); const roles = rolesCopy(root); @@ -39,6 +42,8 @@ export function fixture(root, id = "acme", edit = (doc) => doc) { for (const r of Object.values(base.roles)) r.vars = { harness: "pi", model: "glm-5.3-flash", thinking: "high", ...r.vars }; const doc = edit({ ...base, plugins: { ...base.plugins, harness: ["pi", "claude-code"] } }); writeJson(join(root, "config", "businesses", `${id}.json`), doc); + mkdirSync(join(root, "data"), { recursive: true, mode: 0o700 }); + cpSync(REPO_SKILLS, join(root, "data", "skills-enabled"), { recursive: true }); return { config, roles, dataRoot: join(root, "data"), doc, env: humanEnv({ MOSAIC_CONFIG: config, MOSAIC_ROLES_DIR: roles }) }; } diff --git a/packages/cli/tests/instances.test.mjs b/packages/cli/tests/instances.test.mjs index 33d88b1c2..7c644b42e 100644 --- a/packages/cli/tests/instances.test.mjs +++ b/packages/cli/tests/instances.test.mjs @@ -4,7 +4,7 @@ import test from "node:test"; import assert from "node:assert/strict"; import { spawnSync } from "node:child_process"; -import { mkdirSync, readFileSync } from "node:fs"; +import { readFileSync } from "node:fs"; import { join } from "node:path"; import { fileURLToPath } from "node:url"; import { BusError } from "../../bus/src/broker.mjs"; @@ -20,7 +20,6 @@ const MOSAIC = fileURLToPath(new URL("../../../scripts/mosaic", import.meta.url) async function setup(t) { const root = tmp(t, "mosaic-cli-instances-"); const f = fixture(root, "acme"); - mkdirSync(f.dataRoot, { mode: 0o700 }); const dir = join(root, "config"); const bus = await startBroker({ dataRoot: f.dataRoot, diff --git a/packages/cli/tests/launcher.test.mjs b/packages/cli/tests/launcher.test.mjs index fb0bf47ae..06f4abe49 100644 --- a/packages/cli/tests/launcher.test.mjs +++ b/packages/cli/tests/launcher.test.mjs @@ -11,7 +11,7 @@ import { spawn, spawnSync } from "node:child_process"; import { once } from "node:events"; import { createInterface } from "node:readline"; import { connect } from "node:net"; -import { chmodSync, existsSync, mkdirSync, readFileSync, readdirSync, renameSync, rmSync, statSync, writeFileSync } from "node:fs"; +import { chmodSync, existsSync, mkdirSync, readFileSync, readdirSync, renameSync, rmSync, statSync, symlinkSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { fileURLToPath } from "node:url"; import { Client } from "../../bus/src/client.mjs"; @@ -70,7 +70,6 @@ function prepare(t, more = (doc) => doc) { const f = fixture(root, "acme", (doc) => more(edit(doc))); for (const k of SESSION_CREDENTIALS.flat()) delete f.env[k]; f.env.CLAUDE_CODE_OAUTH_TOKEN = "stand-in"; - mkdirSync(f.dataRoot, { mode: 0o700 }); // The project root is a repository on main, so each launch can clone it. const project = f.doc.projects.stack.root; gitIn(project, "init", "-q", "-b", "main"); @@ -728,6 +727,43 @@ test("a missing or failing bwrap refuses sandbox-unavailable before anything sta assert.deepEqual(readdirSync(join(f.dataRoot, "launches")), ["acme.jsonl"], "no run was made"); }); +test("row 64 a role skill that isn't enabled refuses skill-not-enabled, and one that isn't a plain directory skill-refused, before anything starts", { timeout: 30000 }, async (t) => { + const given = prepare(t); + // The chief of staff's role file names no skill; this copy gives it one, so --chief takes the check too. + const chief = join(given.f.roles, "chief-of-staff.json"); + writeFileSync(chief, JSON.stringify({ ...JSON.parse(readFileSync(chief, "utf8")), skills: ["ms-unslop"] }, null, 2)); + const enabled = join(given.f.dataRoot, "skills-enabled"); + rmSync(join(enabled, "ms-unslop"), { recursive: true }); + const { f, launcher, cto } = await setup(t, undefined, given); + await assert.rejects(launcher.launchPm(), (e) => e.exitCode === 3 && e.message === "PM launch refused: skill-not-enabled (skill ms-unslop isn't enabled; run scripts/skill.sh activate ms-unslop)"); + // The coder's list is ms-unslop, ms-sdlc, ms-file-read, ms-file-write: the first one missing is named. + assert.deepEqual(await launcher.fromSession(cto.cap, "coder"), { ok: false, error: "skill-not-enabled" }); + // A symlink in skills-enabled is refused, not followed. + symlinkSync(join(REPO, "skills", "ms-unslop"), join(enabled, "ms-unslop")); + await assert.rejects(launcher.launchPm(), (e) => e.exitCode === 3 && e.message === `PM launch refused: skill-refused (skill ms-unslop isn't a plain directory under ${enabled})`); + assert.deepEqual(await launcher.fromSession(cto.cap, "coder"), { ok: false, error: "skill-refused" }); + assert.deepEqual(log(f).map((e) => [e.event, e.code]), [["refused", "skill-not-enabled"], ["refused", "skill-not-enabled"], ["refused", "skill-refused"], ["refused", "skill-refused"]]); + assert.deepEqual(readSessions(f.dataRoot), []); + assert.deepEqual(readdirSync(join(f.dataRoot, "launches")), ["acme.jsonl"], "no run was made"); +}); + +test("row 64 on Claude Code a role skill that isn't enabled refuses the same way, before anything starts", { timeout: 30000 }, async (t) => { + const given = prepare(t, (doc) => { + doc.roles.chief.vars.harness = "claude-code"; + doc.roles.coder.vars.harness = "claude-code"; + return doc; + }); + const chief = join(given.f.roles, "chief-of-staff.json"); + writeFileSync(chief, JSON.stringify({ ...JSON.parse(readFileSync(chief, "utf8")), skills: ["ms-unslop"] }, null, 2)); + rmSync(join(given.f.dataRoot, "skills-enabled", "ms-unslop"), { recursive: true }); + const { f, launcher, cto } = await setup(t, undefined, given); + await assert.rejects(launcher.launchPm(), (e) => e.exitCode === 3 && e.message === "PM launch refused: skill-not-enabled (skill ms-unslop isn't enabled; run scripts/skill.sh activate ms-unslop)"); + assert.deepEqual(await launcher.fromSession(cto.cap, "coder"), { ok: false, error: "skill-not-enabled" }); + assert.deepEqual(log(f).map((e) => [e.event, e.code]), [["refused", "skill-not-enabled"], ["refused", "skill-not-enabled"]]); + assert.deepEqual(readSessions(f.dataRoot), []); + assert.deepEqual(readdirSync(join(f.dataRoot, "launches")), ["acme.jsonl"], "no run was made"); +}); + // The brief's one real Claude Code launch: the host's claude in a coder's // sandbox, logged in with a stand-in proxy key against the scripted Messages // API, runs one Bash call that tries the founder's stand-ins, another run diff --git a/packages/cli/tests/thread-launch.test.mjs b/packages/cli/tests/thread-launch.test.mjs index 3c9f076a9..2ef1ea804 100644 --- a/packages/cli/tests/thread-launch.test.mjs +++ b/packages/cli/tests/thread-launch.test.mjs @@ -12,7 +12,7 @@ import { test } from "node:test"; import assert from "node:assert/strict"; import { spawnSync } from "node:child_process"; -import { existsSync, mkdirSync, readdirSync, readFileSync, writeFileSync } from "node:fs"; +import { existsSync, readdirSync, readFileSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { fileURLToPath } from "node:url"; import { launchDoc } from "../../business/tests/helpers.mjs"; @@ -57,7 +57,6 @@ function prepare(t, { sonnet = 2, idle = 1, edit = (doc) => doc } = {}) { return edit(doc); }); for (const k of SESSION_CREDENTIALS.flat()) delete f.env[k]; - mkdirSync(f.dataRoot, { mode: 0o700 }); const project = f.doc.projects.stack.root; gitIn(project, "init", "-q", "-b", "main"); writeFileSync(join(project, "file.txt"), "one\n"); diff --git a/packages/harness/README.md b/packages/harness/README.md index 54cddc4e0..a1677fe0e 100644 --- a/packages/harness/README.md +++ b/packages/harness/README.md @@ -36,7 +36,7 @@ passing it, and shows the memory reaching the model without it. ## The bundle -`buildBundle({dir, resolved, contract, arbiter, tracker, credentials, session, harness, route, routeReason, harnessVersion, models, land})` +`buildBundle({dir, resolved, contract, arbiter, tracker, credentials, session, harness, route, routeReason, harnessVersion, models, land, skillsDir})` writes the bundle once into `dir` (0700; every file 0600, created with `wx`, so an existing bundle is never overwritten): @@ -45,6 +45,18 @@ so an existing bundle is never overwritten): tools). The built-in tools are named as the harness names them, as the manifest lists them: `limits.tools` for Pi, `claudeBuiltins` for Claude Code (Read, Write, Edit, Grep, Glob). + When the role file lists skills, a "Skills" section follows, holding + every file of each listed skill between `` and ``, byte for byte, in sorted path order. +- `skills//`: the bundle's copy of each listed skill, taken from + `skillsDir` (`/skills-enabled`, which `scripts/skill.sh + activate` fills). The prompt text is read from the copy, so a skill + deactivated after the build leaves the run's copy as it was. A listed + skill that isn't enabled refuses `skill-not-enabled`. So does a symlinked + skill, `SKILL.md` or sub-file, a file name outside `[A-Za-z0-9._/-]`, + non-UTF-8 bytes, a file holding ``, or a role whose skills + add up to more than `SKILL_BYTES` (64 KiB; the prompt goes to the harness as + one argv argument, and Linux caps one argument at 128 KiB). - `policy.json`: harness, workspace, built-in tools (`limits.tools`), typed tool names, network, the services the role needs, and the credential status the host read from the broker (metadata only, never a token). @@ -317,8 +329,7 @@ These are limits, not bugs, and nothing in this package claims otherwise. `brokerRetries` times, then exit 23. The next host kills what still runs and ends those launches as `host-lost` (`packages/cli/README.md`, "Sessions"). -- **Resolution** runs without the project layer, there is no skills source - (bundles list none). The route's `thinking` level reaches the adapter as +- **Resolution** runs without the project layer. The route's `thinking` level reaches the adapter as `MOSAIC_THINKING`: pi gets `--thinking`, Claude Code `--effort` (`adapters/README.md`). - **The reply cap is per reply.** Claude Code also persists the largest @@ -339,6 +350,10 @@ These are limits, not bugs, and nothing in this package claims otherwise. changed between the check and the tool's own open (by `bash`, or by another process of the same user) isn't seen. That is the same reach as `bash` itself. +- **Skills are prompt text, not harness skills.** Pi keeps `--no-skills` + and Claude Code keeps `--disable-slash-commands`, so neither harness + discovers or loads a skill itself; the role reads its skills in the + prompt. A skill's scripts or assets are text there too, never run. - **The system prompt is in argv** for both adapters, so the same UID can read it in `/proc//cmdline` outside the sandbox; the PID namespace hides it from other sessions. It holds no secret. diff --git a/packages/harness/src/bundle.mjs b/packages/harness/src/bundle.mjs index 95ee60d20..71cb08f0c 100644 --- a/packages/harness/src/bundle.mjs +++ b/packages/harness/src/bundle.mjs @@ -11,7 +11,7 @@ // names them and the manifest records them. import { createHash } from "node:crypto"; -import { mkdirSync, readFileSync, writeFileSync } from "node:fs"; +import { lstatSync, mkdirSync, readdirSync, readFileSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { fileURLToPath } from "node:url"; import { typedTools, untypedActions } from "./tools.mjs"; @@ -97,6 +97,81 @@ function checkoutLines(checkouts) { return [`- Project clones: ${checkouts.map(one).join("; ")}. Each is a git clone of the project. At a launch the host fetches each clone it can safely run git in from its origin, and fast-forwards only a clean clone on the root's branch, never during a session; the origin is outside this sandbox. Don't set git config in a clone. Any key that \`git clone\` didn't write, user.name and user.email included, makes every later launch keep that clone as it is. Your commits already carry your role instance's name and address from the environment.`]; } +// Row 64 (#1557): a role's skills, by name, from the skills-enabled directory into the bundle. +// The copy is what the session reads, so a deactivate during the session changes nothing. Only +// plain files and directories copy; a symlink, a file that isn't UTF-8 or skills past the size +// limit refuse, as does a file holding the closing marker the prompt puts after it. The limit is +// for all of a role's skills together: they go into prompt.md, and the claude adapter passes +// prompt.md as one argument, which Linux caps at 128 KiB. +export const SKILL_BYTES = 64 * 1024; +const SKILL_END = ""; +const SKILL_PATH = /^[A-Za-z0-9._-]+$/; +const utf8 = new TextDecoder("utf-8", { fatal: true }); +export function skillNotEnabled(name) { + return new BundleError(`skill ${name} isn't enabled; run scripts/skill.sh activate ${name}`, "skill-not-enabled", name); +} +function readSkill(name, root, size) { + const files = []; + const walk = (from, rel) => { + for (const entry of readdirSync(from).sort()) { + const path = join(from, entry), at = rel ? `${rel}/${entry}` : entry; + // The name goes into the marker line, so it keeps to the skill name's characters. + if (!SKILL_PATH.test(entry)) throw new BundleError(`skill ${name}: ${JSON.stringify(at)} isn't a plain file name`); + const stat = lstatSync(path); + if (stat.isDirectory()) { walk(path, at); continue; } + if (!stat.isFile()) throw new BundleError(`skill ${name}: ${at} isn't a plain file or directory`); + const data = readFileSync(path); + size.bytes += data.length; + if (size.bytes > SKILL_BYTES) throw new BundleError(`the role's skills are over ${SKILL_BYTES} bytes together, at skill ${name}`); + let text; + try { text = utf8.decode(data); } catch { throw new BundleError(`skill ${name}: ${at} isn't UTF-8 text`); } + if (text.includes(SKILL_END)) throw new BundleError(`skill ${name}: ${at} holds ${SKILL_END}`); + files.push({ rel: at, text }); + } + }; + const found = (path) => { + try { return lstatSync(path); } catch (e) { if (e.code === "ENOENT") throw skillNotEnabled(name); throw e; } + }; + // scripts/skill.sh activate moves a real directory in, so a symlink here is refused, not followed. + if (!found(join(root, name)).isDirectory()) throw new BundleError(`skill ${name} isn't a plain directory under ${root}`); + if (!found(join(root, name, "SKILL.md")).isFile()) throw new BundleError(`skill ${name}: SKILL.md isn't a plain file`); + walk(join(root, name), ""); + return files; +} +// Every named skill read and checked, or a BundleError: code skill-not-enabled names the first +// skill with no SKILL.md under root, bundle-refused anything else. The launcher calls this before +// the run exists, and buildBundle reads again for the copy. +export function readSkills(names, root) { + if (!names.length) return []; + if (!root) throw new BundleError("the role has skills, but the bundle has no skills directory"); + const size = { bytes: 0 }; + return names.map((name) => ({ name, files: readSkill(name, root, size) })); +} +function copySkills(names, root, dir) { + return readSkills(names, root).map(({ name, files }) => { + const base = join(dir, "skills", name); + mkdirSync(base, { recursive: true, mode: 0o700 }); + for (const f of files) { + const parts = f.rel.split("/"); + if (parts.length > 1) mkdirSync(join(base, ...parts.slice(0, -1)), { recursive: true, mode: 0o700 }); + writeFileSync(join(base, ...parts), f.text, { mode: 0o600, flag: "wx" }); + } + const digests = Object.fromEntries(files.map((f) => [f.rel, sha256(f.text)])); + return { name, path: base, sha256: digests["SKILL.md"], files: digests, contents: files }; + }); +} +// Each file in full between its markers: the opening line, the file's bytes, a newline, then the +// closing marker on its own line. Taking out the text between the opening line and "\n" +// gives the bundle copy byte for byte. +function skillSection(skills) { + if (!skills.length) return ""; + const parts = ["", "## Skills", "", `Your role's skills: ${skills.map((s) => s.name).join(", ")}. Each file is shown in full between its markers. A skill guides how you work; it grants no tool or action this session doesn't already hold.`, ""]; + for (const s of skills) { + for (const f of s.contents) parts.push(`\n${f.text}\n${SKILL_END}`, ""); + } + return parts.join("\n"); +} + function prompt({ contract, resolved, session, tools, untyped, builtins }) { const lines = [ contract.trimEnd(), @@ -137,7 +212,7 @@ function prompt({ contract, resolved, session, tools, untyped, builtins }) { return lines.join("\n") + "\n"; } -// buildBundle({ dir, resolved, contract, arbiter, tracker, credentials, session, harness, route, routeReason, harnessVersion, models, land }) +// buildBundle({ dir, resolved, contract, arbiter, tracker, credentials, session, harness, route, routeReason, harnessVersion, models, land, skillsDir }) // dir the bundle directory, created here (0700) // resolved resolveInstance(...) output // contract the role contract's text @@ -153,15 +228,18 @@ function prompt({ contract, resolved, session, tools, untyped, builtins }) { // harnessVersion the module's version(), for the manifest // models launch.models // land true when the role and thread hold a merge decision to land (row 137) +// skillsDir /skills-enabled, where each of resolved.skills is read from (row 64); +// a role with skills and no skillsDir refuses // Returns { manifest, files, policy, tools, env } with absolute paths; env is the module's // adapter variables, which the host writes into session.json. -export function buildBundle({ dir, resolved, contract, arbiter = false, tracker = false, credentials = [], session, harness, route = "primary", routeReason = null, harnessVersion = null, models = null, land = false }) { +export function buildBundle({ dir, resolved, contract, arbiter = false, tracker = false, credentials = [], session, harness, route = "primary", routeReason = null, harnessVersion = null, models = null, land = false, skillsDir = null }) { if (route !== "primary" && route !== "fallback") throw new BundleError(`route ${JSON.stringify(route)} isn't primary or fallback`); const model = sessionModel(resolved, models, route); const web = webPlan(resolved, model.harness, harness.builtinWeb()); const tools = typedTools({ resolved, arbiter, tracker, projects: (session.checkouts ?? []).map((c) => c.project), webTyped: !web.builtin, land }); const untyped = untypedActions(resolved, tools); mkdirSync(dir, { recursive: true, mode: 0o700 }); + const skills = copySkills(resolved.skills ?? [], skillsDir, dir); const write = (name, text) => { writeFileSync(join(dir, name), text, { mode: 0o600, flag: "wx" }); return [name, sha256(text)]; @@ -184,7 +262,7 @@ export function buildBundle({ dir, resolved, contract, arbiter = false, tracker }; const builtins = harness.builtins({ tools: policy.tools, webBuiltin: web.builtin }); const digests = [ - write("prompt.md", prompt({ contract, resolved, session: { ...session, ...model, webBuiltin: web.builtin }, tools, untyped, builtins })), + write("prompt.md", prompt({ contract, resolved, session: { ...session, ...model, webBuiltin: web.builtin }, tools, untyped, builtins }) + skillSection(skills)), write("policy.json", JSON.stringify(policy, null, 2) + "\n"), write("tools.json", JSON.stringify(tools, null, 2) + "\n"), ]; @@ -209,7 +287,7 @@ export function buildBundle({ dir, resolved, contract, arbiter = false, tracker typed: tools.map((t) => harness.typedName(t.name)), untyped, webSource: resolved.limits.tools.includes("web") ? web.source : null, - skills: [], + skills: skills.map(({ name, path, sha256, files }) => ({ name, path, sha256, files })), reliesOn: [...harness.reliesOn()], gateTimeout: harness.gateTimeout(), files: Object.fromEntries(digests), diff --git a/packages/harness/tests/bundle.test.mjs b/packages/harness/tests/bundle.test.mjs index 677ec1561..7690d9761 100644 --- a/packages/harness/tests/bundle.test.mjs +++ b/packages/harness/tests/bundle.test.mjs @@ -4,7 +4,7 @@ import { createHash } from "node:crypto"; import { mkdirSync, readFileSync, statSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { BundleError, buildBundle, sessionModel, SOURCES } from "../src/bundle.mjs"; -import { HARNESS, resolvedFor, scratch } from "./helpers.mjs"; +import { HARNESS, REPO_SKILLS, resolvedFor, scratch } from "./helpers.mjs"; import { decide } from "../src/gate.mjs"; const sha = (p) => createHash("sha256").update(readFileSync(p)).digest("hex"); @@ -23,6 +23,7 @@ function build(t, resolved, extra = {}) { session: { run: "run-1", launchedBy: "pm", workspace, toolSocket: join(dir, "tools.sock"), turnMarker: join(dir, "turn.done") }, harness: HARNESS[sessionModel(resolved, null, extra.route ?? "primary").harness], harnessVersion: "fixture-version", + skillsDir: REPO_SKILLS, ...extra, }); return { dir, workspace, ...bundle }; @@ -76,7 +77,7 @@ test("a pi bundle: prompt, policy, tools and manifest, 0600 in a 0700 directory" assert.equal(m.thinking, "high"); assert.deepEqual([m.route, m.routeReason, m.harnessVersion], ["primary", null, "fixture-version"]); assert.equal(m.gateTimeout, null); - assert.deepEqual(m.skills, []); + assert.deepEqual(m.skills.map((x) => x.name), ["ms-unslop", "ms-goal", "ms-communications"]); assert.equal(m.reliesOn.length, 5); assert.ok(!m.typed.includes("launch"), "row 119: the PM no longer launches"); assert.deepEqual(Object.keys(m.sources).sort(), ["gate", "piExtension", "tools"]); @@ -229,6 +230,7 @@ test("a bundle is written once: an existing file refuses", (t) => { contract: "c", session: { run: "r", launchedBy: "pm", workspace: dir, toolSocket: join(dir, "s"), turnMarker: join(dir, "m") }, harness: HARNESS.pi, + skillsDir: REPO_SKILLS, }), /EEXIST/); assert.equal(readFileSync(join(dir, "bundle", "prompt.md"), "utf8"), "old"); }); @@ -242,6 +244,7 @@ test("a path with a single quote can't go into the hook command", (t) => { contract: "c", session: { run: "r", launchedBy: "pm", workspace: dir, toolSocket: join(dir, "s"), turnMarker: join(dir, "m") }, harness: HARNESS["claude-code"], + skillsDir: REPO_SKILLS, }), (e) => e instanceof BundleError && /single quote/.test(e.message)); }); diff --git a/packages/harness/tests/claude-session.test.mjs b/packages/harness/tests/claude-session.test.mjs index 02e59a83e..19e9f2f6d 100644 --- a/packages/harness/tests/claude-session.test.mjs +++ b/packages/harness/tests/claude-session.test.mjs @@ -11,7 +11,7 @@ import { chmodSync, existsSync, mkdirSync, readFileSync, realpathSync, rmSync, w import { basename, dirname, join } from "node:path"; import { buildBundle } from "../src/bundle.mjs"; import { adapterEnv } from "../src/runner.mjs"; -import { fakeToolSocket, HARNESS, mockAnthropic, REPO, resolvedFor, scratch } from "./helpers.mjs"; +import { fakeToolSocket, HARNESS, mockAnthropic, REPO, REPO_SKILLS, resolvedFor, scratch } from "./helpers.mjs"; const ADAPTER = join(REPO, "adapters", "claude", "adapter.sh"); const which = spawnSync("sh", ["-c", "command -v claude"], { encoding: "utf8" }); @@ -42,7 +42,7 @@ async function session(t, script, tools = ["read", "bash"]) { toolSocket: sock.path, turnMarker: join(dir, "turn.done"), }; - const { files, manifest, env: harnessEnv } = buildBundle({ dir: join(dir, "bundle"), resolved, contract: "# Coder\n", harness: HARNESS["claude-code"], session: { ...s, run: "run-1", launchedBy: "pm" } }); + const { files, manifest, env: harnessEnv } = buildBundle({ dir: join(dir, "bundle"), resolved, contract: "# Coder\n", harness: HARNESS["claude-code"], skillsDir: REPO_SKILLS, session: { ...s, run: "run-1", launchedBy: "pm" } }); s.bundle = files; s.harnessEnv = harnessEnv; const env = adapterEnv(s, { diff --git a/packages/harness/tests/helpers.mjs b/packages/harness/tests/helpers.mjs index e4bdb3058..6afc4910f 100644 --- a/packages/harness/tests/helpers.mjs +++ b/packages/harness/tests/helpers.mjs @@ -13,6 +13,9 @@ import { businessDoc, REPO_ROLES, systemFor } from "../../business/tests/helpers import { createLoader } from "../../plugins/src/loader.mjs"; export { REPO } from "../../business/tests/helpers.mjs"; +import { REPO } from "../../business/tests/helpers.mjs"; +// Row 64: the repository's skill directories, as /skills-enabled holds them once activated. +export const REPO_SKILLS = join(REPO, "skills"); export function scratch(t, prefix = "mosaic-harness-") { const dir = realpathSync(mkdtempSync(join(tmpdir(), prefix))); diff --git a/packages/harness/tests/pi-session.test.mjs b/packages/harness/tests/pi-session.test.mjs index 56005f865..ceda82beb 100644 --- a/packages/harness/tests/pi-session.test.mjs +++ b/packages/harness/tests/pi-session.test.mjs @@ -9,7 +9,7 @@ import { join } from "node:path"; import { buildBundle } from "../src/bundle.mjs"; import { adapterEnv } from "../src/runner.mjs"; import { REPLY_CAP } from "../src/tools.mjs"; -import { fakeToolSocket, HARNESS, mockAnthropic, REPO, resolvedFor, scratch } from "./helpers.mjs"; +import { fakeToolSocket, HARNESS, mockAnthropic, REPO, REPO_SKILLS, resolvedFor, scratch } from "./helpers.mjs"; const ADAPTER = join(REPO, "adapters", "pi", "adapter.sh"); @@ -44,7 +44,7 @@ async function session(t, script, place = (dir) => join(dir, "ws"), instance = " turnMarker: join(dir, "turn.done"), }; const models = { "probe-model": { provider: "probe", model: "probe-model" } }; - const { files, env: harnessEnv } = buildBundle({ dir: join(dir, "bundle"), resolved, contract: "# PM\n", tracker: true, harness: HARNESS.pi, models, session: { ...s, run: "run-1", launchedBy: "jason" } }); + const { files, env: harnessEnv } = buildBundle({ dir: join(dir, "bundle"), resolved, contract: "# PM\n", tracker: true, harness: HARNESS.pi, models, skillsDir: REPO_SKILLS, session: { ...s, run: "run-1", launchedBy: "jason" } }); s.bundle = files; s.harnessEnv = harnessEnv; const env = adapterEnv(s, { diff --git a/packages/harness/tests/skills.test.mjs b/packages/harness/tests/skills.test.mjs new file mode 100644 index 000000000..3a920763a --- /dev/null +++ b/packages/harness/tests/skills.test.mjs @@ -0,0 +1,209 @@ +// Row 64 (#1557): a role's skills reach the session as prompt text. The bundle copies each +// skill on the role's list from /skills-enabled, and prompt.md carries every copied +// file between markers, byte for byte. Neither harness discovers a skill on its own. + +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; +import { createHash } from "node:crypto"; +import { chmodSync, cpSync, mkdirSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { BundleError, buildBundle, readSkills, SKILL_BYTES } from "../src/bundle.mjs"; +import { adapterEnv } from "../src/runner.mjs"; +import { HARNESS, REPO, REPO_SKILLS, resolvedFor, scratch } from "./helpers.mjs"; + +const sha = (text) => createHash("sha256").update(text).digest("hex"); +const MARKED = /\n([\s\S]*?)\n<\/skill-file>/g; + +function build(t, resolved, skillsDir = REPO_SKILLS) { + const dir = scratch(t); + const workspace = join(dir, "ws"); + mkdirSync(workspace); + const harness = HARNESS[resolved.vars.harness]; + const bundle = buildBundle({ + dir: join(dir, "bundle"), + resolved, + contract: "# Contract\n", + session: { run: "run-1", launchedBy: "pm", workspace, toolSocket: join(dir, "tools.sock"), turnMarker: join(dir, "turn.done") }, + harness, + skillsDir, + }); + return { dir, ...bundle, prompt: readFileSync(bundle.files.prompt, "utf8") }; +} + +// A skills-enabled directory in scratch holding copies of the named repo skills. +function enabled(t, names) { + const root = join(scratch(t, "mosaic-skills-"), "skills-enabled"); + mkdirSync(root); + for (const name of names) cpSync(join(REPO_SKILLS, name), join(root, name), { recursive: true }); + return root; +} + +const withSkills = (t, skills, instance = "reviewer") => ({ ...resolvedFor(t, instance), skills }); +const refused = (pattern, code = "bundle-refused") => (e) => e instanceof BundleError && e.code === code && pattern.test(e.message); + +test("each repo role's prompt carries exactly its listed skills, each file as the bundle copy and the source", (t) => { + for (const instance of ["coder", "reviewer", "pm", "cto"]) { + for (const harness of ["pi", "claude-code"]) { + const r = resolvedFor(t, instance, (d) => (d.roles[instance].vars = { harness, model: "claude-opus-5-5", thinking: "high" })); + assert.ok(r.skills.length > 0, instance); + const b = build(t, r); + const marked = [...b.prompt.matchAll(MARKED)].map(([, name, rel, text]) => ({ name, rel, text })); + assert.deepEqual([...new Set(marked.map((m) => m.name))], r.skills, `${instance} ${harness}`); + assert.match(b.prompt, new RegExp(`\\n## Skills\\n\\nYour role's skills: ${r.skills.join(", ")}\\. `)); + assert.deepEqual(b.manifest.skills.map((s) => s.name), r.skills); + for (const s of b.manifest.skills) { + assert.equal(s.path, join(b.dir, "bundle", "skills", s.name)); + assert.equal(s.sha256, s.files["SKILL.md"]); + const files = marked.filter((m) => m.name === s.name); + assert.deepEqual(files.map((f) => f.rel), Object.keys(s.files), s.name); + for (const f of files) { + const copy = readFileSync(join(s.path, f.rel), "utf8"); + assert.equal(f.text, copy, `${s.name}/${f.rel} prompt vs copy`); + assert.equal(copy, readFileSync(join(REPO_SKILLS, s.name, f.rel), "utf8"), `${s.name}/${f.rel} copy vs source`); + assert.equal(s.files[f.rel], sha(copy), `${s.name}/${f.rel} digest`); + } + } + assert.deepEqual(JSON.parse(readFileSync(b.files.manifest, "utf8")).skills, JSON.parse(JSON.stringify(b.manifest.skills))); + } + } +}); + +test("a skill's files go into the prompt in sorted path order, sub-directories included", (t) => { + const b = build(t, withSkills(t, ["ms-grill-me"])); + assert.deepEqual([...b.prompt.matchAll(MARKED)].map((m) => m[2]), ["SKILL.md", "upstream-grill-me.md", "upstream-grilling.md"]); + const goal = build(t, withSkills(t, ["ms-goal"])); + assert.deepEqual([...goal.prompt.matchAll(MARKED)].map((m) => m[2]), ["SKILL.md", "references/execution-checks.md"]); +}); + +test("a role with no skills gets no Skills section, no skills directory and an empty manifest list", (t) => { + for (const harness of ["pi", "claude-code"]) { + const r = resolvedFor(t, "chief", (d) => (d.roles.chief.vars = { harness, model: "claude-opus-5-5" })); + assert.deepEqual(r.skills, []); + for (const skillsDir of [REPO_SKILLS, null]) { + const b = build(t, r, skillsDir); + assert.doesNotMatch(b.prompt, /## Skills| { + const root = enabled(t, ["ms-unslop", "ms-sdlc", "ms-file-read", "ms-archify"]); + for (const harness of ["pi", "claude-code"]) { + const b = build(t, resolvedFor(t, "reviewer", (d) => (d.roles.reviewer.vars = { harness, model: "claude-opus-5-5", thinking: "high" })), root); + assert.doesNotMatch(b.prompt, /ms-archify/, harness); + assert.deepEqual(b.manifest.skills.map((s) => s.name), ["ms-unslop", "ms-sdlc", "ms-file-read"], harness); + } +}); + +test("the bundle copy outlives the source: deactivating a skill after the build leaves the run's copy", (t) => { + const root = enabled(t, ["ms-unslop", "ms-sdlc", "ms-file-read"]); + const b = build(t, resolvedFor(t, "reviewer"), root); + rmSync(join(root, "ms-sdlc"), { recursive: true }); + const s = b.manifest.skills.find((x) => x.name === "ms-sdlc"); + assert.equal(sha(readFileSync(join(s.path, "SKILL.md"), "utf8")), s.sha256); +}); + +test("a listed skill that isn't enabled refuses skill-not-enabled, naming the activate command", (t) => { + const root = enabled(t, ["ms-unslop", "ms-file-read"]); + for (const harness of ["pi", "claude-code"]) { + const r = resolvedFor(t, "reviewer", (d) => (d.roles.reviewer.vars = { harness, model: "claude-opus-5-5", thinking: "high" })); + assert.throws(() => build(t, r, root), refused(/^skill ms-sdlc isn't enabled; run scripts\/skill\.sh activate ms-sdlc$/, "skill-not-enabled"), harness); + } + assert.throws(() => readSkills(["ms-sdlc"], root), (e) => e.code === "skill-not-enabled" && e.missing === "ms-sdlc"); + const noFile = enabled(t, []); + mkdirSync(join(noFile, "ms-sdlc")); + assert.throws(() => readSkills(["ms-sdlc"], noFile), refused(/^skill ms-sdlc isn't enabled/, "skill-not-enabled")); + assert.throws(() => build(t, resolvedFor(t, "reviewer"), null), refused(/the role has skills, but the bundle has no skills directory/)); +}); + +test("a skill directory that isn't plain refuses: a symlinked skill, SKILL.md or sub-file, or a non-file entry", (t) => { + const real = enabled(t, ["ms-sdlc"]); + const linkedDir = enabled(t, []); + symlinkSync(join(real, "ms-sdlc"), join(linkedDir, "ms-sdlc")); + assert.throws(() => readSkills(["ms-sdlc"], linkedDir), refused(/^skill ms-sdlc isn't a plain directory under /)); + const linkedSkill = enabled(t, []); + mkdirSync(join(linkedSkill, "ms-sdlc")); + symlinkSync(join(real, "ms-sdlc", "SKILL.md"), join(linkedSkill, "ms-sdlc", "SKILL.md")); + assert.throws(() => readSkills(["ms-sdlc"], linkedSkill), refused(/^skill ms-sdlc: SKILL\.md isn't a plain file$/)); + const linkedSub = enabled(t, ["ms-sdlc"]); + symlinkSync("/etc/hostname", join(linkedSub, "ms-sdlc", "extra.md")); + assert.throws(() => readSkills(["ms-sdlc"], linkedSub), refused(/^skill ms-sdlc: extra\.md isn't a plain file or directory$/)); +}); + +test("a skill file that can't go into the prompt refuses: a bad name, non-UTF-8 bytes, or the end marker", (t) => { + const cases = [ + ["has space.md", "text\n", /"references\/has space\.md" isn't a plain file name/], + ['q".md', "text\n", /"references\/q\\"\.md" isn't a plain file name/], + ["bytes.md", Buffer.from([0x66, 0xff, 0x0a]), /bytes\.md isn't UTF-8 text/], + ["marker.md", "before\n\nafter\n", /marker\.md holds <\/skill-file>/], + ]; + for (const [file, data, pattern] of cases) { + const root = enabled(t, ["ms-sdlc"]); + mkdirSync(join(root, "ms-sdlc", "references")); + writeFileSync(join(root, "ms-sdlc", "references", file), data); + assert.throws(() => readSkills(["ms-sdlc"], root), refused(pattern), file); + } +}); + +test("a role's skills over SKILL_BYTES together refuse, naming the skill that crossed it", (t) => { + const root = enabled(t, []); + const half = "x".repeat(SKILL_BYTES / 2 - 8) + "\n"; + for (const name of ["s-one", "s-two", "s-three"]) { + mkdirSync(join(root, name)); + writeFileSync(join(root, name, "SKILL.md"), half); + } + assert.equal(readSkills(["s-one"], root)[0].files[0].text, half); + assert.equal(readSkills(["s-one", "s-two"], root).length, 2); + assert.throws(() => readSkills(["s-one", "s-two", "s-three"], root), refused(new RegExp(`^the role's skills are over ${SKILL_BYTES} bytes together, at skill s-three$`))); + // Each repo role's list fits. + for (const instance of ["coder", "reviewer", "pm", "cto"]) { + assert.doesNotThrow(() => readSkills(resolvedFor(t, instance).skills, REPO_SKILLS), instance); + } +}); + +// Mos's check (mos-0857): the adapter hands the harness the prompt with the skills in it, and +// discovery stays off. A stand-in for each harness's CLI records the adapter's argv, NUL-separated. +// The chief of staff lists no skill: its prompt carries none and discovery is off all the same. +test("each adapter passes the skills only as --system-prompt text, with skill discovery off", (t) => { + for (const [instance, harness] of [["reviewer", "pi"], ["reviewer", "claude-code"], ["chief", "pi"], ["chief", "claude-code"]]) { + const dir = scratch(t); + const workspace = join(dir, "ws"); + mkdirSync(workspace); + mkdirSync(join(dir, "bin")); + const cli = join(dir, "bin", harness === "pi" ? "pi" : "claude"); + writeFileSync(cli, '#!/bin/sh\nprintf "%s\\0" "$@" > "$ARGV_OUT"\necho ok\n'); + chmodSync(cli, 0o755); + const resolved = resolvedFor(t, instance, (d) => (d.roles[instance].vars = { harness, model: "claude-opus-5-5", thinking: "high" })); + assert.equal(resolved.skills.length > 0, instance === "reviewer", instance); + const s = { harness, provider: harness === "pi" ? "probe" : null, model: "claude-opus-5-5", workspace, sessionDir: join(dir, "session"), toolSocket: join(dir, "tools.sock"), turnMarker: join(dir, "turn.done") }; + const models = { "claude-opus-5-5": { provider: "probe", model: "claude-opus-5-5" } }; + const b = buildBundle({ dir: join(dir, "bundle"), resolved, contract: "# Contract\n", harness: HARNESS[harness], models, skillsDir: REPO_SKILLS, session: { ...s, run: "run-1", launchedBy: "pm" } }); + s.bundle = b.files; + s.harnessEnv = b.env; + const env = { ...adapterEnv(s, { PATH: `${join(dir, "bin")}:/usr/bin:/bin`, HOME: join(dir, "home") }), ARGV_OUT: join(dir, "argv"), MOSAIC_REQUEST: "x" }; + const r = spawnSync("/bin/sh", [join(REPO, "adapters", harness === "pi" ? "pi" : "claude", "adapter.sh")], { encoding: "utf8", stdio: ["ignore", "pipe", "pipe"], env }); + assert.equal(r.status, 0, `${harness}: ${r.stderr}`); + const argv = readFileSync(join(dir, "argv"), "utf8").split("\0"); + const i = argv.indexOf("--system-prompt"); + assert.ok(i >= 0 && argv.indexOf("--system-prompt", i + 1) < 0, harness); + // The adapter reads the file with $(cat ...), which drops trailing newlines. + assert.equal(argv[i + 1], readFileSync(b.files.prompt, "utf8").replace(/\n+$/, ""), harness); + const marked = [...argv[i + 1].matchAll(MARKED)]; + assert.deepEqual([...new Set(marked.map((m) => m[1]))], resolved.skills, harness); + if (instance === "chief") assert.doesNotMatch(argv[i + 1], /## Skills| k !== i + 1); + assert.ok(!rest.some((a) => a.includes(join(dir, "bundle", "skills")) || a.includes(REPO_SKILLS) || /SKILL\.md| new Promise((reso test('row 165 Console: the signed-in human adds an instance through a real broker; validate exits 0 and the log names the human', async t => { const root = tmp(t, 'webui-instances-'); const f = fixture(root, 'acme'); - mkdirSync(f.dataRoot, { mode: 0o700 }); const dir = join(root, 'config'); const host = await startBroker({ dataRoot: f.dataRoot, diff --git a/packages/webui/tests/routes-browser.test.mjs b/packages/webui/tests/routes-browser.test.mjs index 80e33839a..c3749858a 100644 --- a/packages/webui/tests/routes-browser.test.mjs +++ b/packages/webui/tests/routes-browser.test.mjs @@ -6,7 +6,7 @@ import test from 'node:test'; import assert from 'node:assert/strict'; import {request} from 'node:http'; -import {mkdirSync,readFileSync} from 'node:fs'; +import {readFileSync} from 'node:fs'; import {join} from 'node:path'; import {browser} from './browser.mjs'; import {startServer} from '../src/serve.mjs'; @@ -29,7 +29,6 @@ test('row 65 Settings: the routing table lists every instance, saves a harness, const root=tmp(t,'webui-routes-browser-'); // The reviewer starts with a fallback, so the table draws a set one as well as an empty one. const f=fixture(root,'acme',doc=>{doc.roles.reviewer.vars={...doc.roles.reviewer.vars,'fallback.harness':'pi','fallback.model':'glm-5.3-flash','fallback.thinking':'low'};return doc;}); - mkdirSync(f.dataRoot,{mode:0o700}); const dir=join(root,'config'),file=join(dir,'businesses','acme.json'); const host=await startBroker({ dataRoot:f.dataRoot, diff --git a/packages/webui/tests/routes.test.mjs b/packages/webui/tests/routes.test.mjs index fa4f389a5..9bcb81138 100644 --- a/packages/webui/tests/routes.test.mjs +++ b/packages/webui/tests/routes.test.mjs @@ -5,7 +5,7 @@ import test from 'node:test'; import assert from 'node:assert/strict'; import { request } from 'node:http'; -import { mkdirSync, readFileSync } from 'node:fs'; +import { readFileSync } from 'node:fs'; import { join } from 'node:path'; import { startServer } from '../src/serve.mjs'; import { BusReadError } from '../src/bus.mjs'; @@ -41,7 +41,6 @@ const get = (s, path) => send(s, path, undefined, { host: 'console.example', 'x- test('row 65 Console: the signed-in human saves a route through a real broker; each refusal says nothing changed and leaves the file and the log', async t => { const root = tmp(t, 'webui-routes-'); const f = fixture(root, 'acme'); - mkdirSync(f.dataRoot, { mode: 0o700 }); const dir = join(root, 'config'); const host = await startBroker({ dataRoot: f.dataRoot, diff --git a/roles/coder.json b/roles/coder.json index eedb4fec3..eb5470227 100644 --- a/roles/coder.json +++ b/roles/coder.json @@ -5,6 +5,7 @@ "contract": "coder.md", "tools": ["read", "write", "edit", "bash", "grep", "find", "ls"], "network": "api-only", + "skills": ["ms-unslop", "ms-sdlc", "ms-file-read", "ms-file-write"], "authority": { "withinRole": ["task.update.assigned", "git.push.working", "review.request", "message.send", "record.write"], "crossRole": ["task.scope.change"] diff --git a/roles/cto.json b/roles/cto.json index afc3f22ed..193770880 100644 --- a/roles/cto.json +++ b/roles/cto.json @@ -5,6 +5,7 @@ "contract": "cto.md", "tools": ["read", "write", "edit", "bash", "grep", "find", "ls"], "network": "api-only", + "skills": ["ms-unslop", "ms-sdlc", "ms-archify", "ms-grill-me"], "authority": { "withinRole": ["review.request", "task.update.assigned", "message.send", "decision.resolve.technical", "record.write"], "crossRole": ["task.scope.change"] diff --git a/roles/pm.json b/roles/pm.json index 1323a87a0..b1d549640 100644 --- a/roles/pm.json +++ b/roles/pm.json @@ -5,6 +5,7 @@ "contract": "pm.md", "tools": ["read", "write", "edit", "grep", "find", "ls"], "network": "api-only", + "skills": ["ms-unslop", "ms-goal", "ms-communications"], "authority": { "withinRole": ["task.create", "task.assign", "task.reassign", "task.schedule", "task.update.assigned", "task.close", "message.send", "record.write"], "crossRole": ["task.priority.change", "task.scope.change"] diff --git a/roles/reviewer.json b/roles/reviewer.json index 6f0969d18..d33f2eda6 100644 --- a/roles/reviewer.json +++ b/roles/reviewer.json @@ -5,6 +5,7 @@ "contract": "reviewer.md", "tools": ["read", "grep", "find", "ls", "bash"], "network": "api-only", + "skills": ["ms-unslop", "ms-sdlc", "ms-file-read"], "authority": { "withinRole": ["review.verdict", "message.send", "task.update.assigned", "record.write"], "crossRole": []