From 01a1d3ca8208a59e5ac101a0d9a4f7f5aa443480 Mon Sep 17 00:00:00 2001 From: Drew Stone Date: Fri, 24 Jul 2026 17:33:55 -0600 Subject: [PATCH] fix(skills-placement): require opt-in for inline skill-body fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit composeSkillsForHarness silently fell back to inline delivery — rendering every skill BODY into the system prompt — whenever a harness had no native skill-mount directory. Of the 14 KNOWN_HARNESSES, 7 (amp, factory-droids, hermes, forge, acp, cursor, cli-base) hit this path, and one product reproduced a 151,882-byte prompt this way. An AgentProfile is a recipe that materializes into a sandbox, not a prompt template: skill bodies belong in a file mount the agent reads on demand, not concatenated into prompt.systemPrompt. Inline delivery is a narrow fallback, never a co-equal mode. composeSkillsForHarness now takes onNoSkillDir ('throw' | 'inline'), defaulting to 'throw'. The thrown error names the harness, the skill count, the byte size that would have been inlined, and the harnesses that support native mounting. Callers that genuinely need inline delivery opt in explicitly with onNoSkillDir: 'inline'. The low-level composeSkills already required an explicit mode; this closes the one recommended integration point that auto-fell back around it. --- docs/api/skills-placement.md | 2 +- docs/codemap.json | 2 +- docs/llms-full.txt | 2 +- src/skills-placement/index.test.ts | 57 +++++++++++++++++++-- src/skills-placement/index.ts | 80 +++++++++++++++++++++++++----- src/skills/index.ts | 3 +- 6 files changed, 126 insertions(+), 20 deletions(-) diff --git a/docs/api/skills-placement.md b/docs/api/skills-placement.md index ea0ce49..e8410af 100644 --- a/docs/api/skills-placement.md +++ b/docs/api/skills-placement.md @@ -16,7 +16,7 @@ interface ComposedSkills ### `composeSkillsForHarness` -`function` — Compose {@link ComposedSkills} for `harness`: `mounted` delivery when the platform names a cwd skill dir for it, `inline` delivery (the automatic fallback that keeps every skill available on every ha… +`function` — Compose {@link ComposedSkills} for `harness`. ```ts (input: ComposeSkillsForHarnessInput) => ComposedSkills diff --git a/docs/codemap.json b/docs/codemap.json index 02a5243..cc52cb8 100644 --- a/docs/codemap.json +++ b/docs/codemap.json @@ -15187,7 +15187,7 @@ "name": "composeSkillsForHarness", "kind": "function", "signature": "(input: ComposeSkillsForHarnessInput) => ComposedSkills", - "doc": "Compose {@link ComposedSkills} for `harness`: `mounted` delivery when the platform names a cwd skill dir for it, `inline` delivery (the automatic fallback that keeps every skill available on every ha…" + "doc": "Compose {@link ComposedSkills} for `harness`." }, { "name": "ComposeSkillsForHarnessInput", diff --git a/docs/llms-full.txt b/docs/llms-full.txt index bcd5404..b77b360 100644 --- a/docs/llms-full.txt +++ b/docs/llms-full.txt @@ -19778,7 +19778,7 @@ interface ComposedSkills ### `composeSkillsForHarness` -`function` — Compose {@link ComposedSkills} for `harness`: `mounted` delivery when the platform names a cwd skill dir for it, `inline` delivery (the automatic fallback that keeps every skill available on every ha… +`function` — Compose {@link ComposedSkills} for `harness`. ```ts (input: ComposeSkillsForHarnessInput) => ComposedSkills diff --git a/src/skills-placement/index.test.ts b/src/skills-placement/index.test.ts index b811792..4370f74 100644 --- a/src/skills-placement/index.test.ts +++ b/src/skills-placement/index.test.ts @@ -67,14 +67,63 @@ describe.skipIf(!available)('composeSkillsForHarness', () => { expect(result.promptSection).not.toContain('Strip the tells.') }) - it('hermes -> inline: no refs, bodies rendered into the section', () => { - const result = mod!.composeSkillsForHarness({ skills, harness: 'hermes' }) + it('hermes, default onNoSkillDir -> throws instead of silently inlining', () => { + expect(() => mod!.composeSkillsForHarness({ skills, harness: 'hermes' })).toThrow( + /no native skill-mount directory/, + ) + }) + + it('amp, default onNoSkillDir -> throws instead of silently inlining', () => { + expect(() => mod!.composeSkillsForHarness({ skills, harness: 'amp' })).toThrow( + /no native skill-mount directory/, + ) + }) + + it('amp, default onNoSkillDir -> throws AND the thrown message never contains a skill body', () => { + // Regression guard for the silent-inline bug: it must actually throw (not + // just fail to contain the body because it silently returned a prompt + // section as before), and the ERROR path itself must never leak skill + // bytes into anything a caller might log/display as if it were a normal + // composed prompt section. + expect(() => mod!.composeSkillsForHarness({ skills, harness: 'amp' })).toThrow() + let message = '' + try { + mod!.composeSkillsForHarness({ skills, harness: 'amp' }) + } catch (err) { + message = (err as Error).message + } + expect(message).not.toBe('') + expect(message).not.toContain('Strip the tells.') + }) + + it('amp, default onNoSkillDir -> error names the harness, skill count, byte size, and mountable harnesses', () => { + expect(() => mod!.composeSkillsForHarness({ skills, harness: 'amp' })).toThrow( + /"amp".*1 skill\(s\).*\d+ bytes.*opencode/s, + ) + }) + + it('amp, onNoSkillDir: "throw" (explicit) -> same as default', () => { + expect(() => + mod!.composeSkillsForHarness({ skills, harness: 'amp', onNoSkillDir: 'throw' }), + ).toThrow(/no native skill-mount directory/) + }) + + it("amp, onNoSkillDir: 'inline' -> explicit opt-in still works: no refs, bodies rendered into the section", () => { + const result = mod!.composeSkillsForHarness({ + skills, + harness: 'amp', + onNoSkillDir: 'inline', + }) expect(result.refs).toEqual([]) expect(result.promptSection).toContain('Strip the tells.') }) - it('amp -> inline: no refs, bodies rendered into the section', () => { - const result = mod!.composeSkillsForHarness({ skills, harness: 'amp' }) + it("hermes, onNoSkillDir: 'inline' -> explicit opt-in still works: no refs, bodies rendered into the section", () => { + const result = mod!.composeSkillsForHarness({ + skills, + harness: 'hermes', + onNoSkillDir: 'inline', + }) expect(result.refs).toEqual([]) expect(result.promptSection).toContain('Strip the tells.') }) diff --git a/src/skills-placement/index.ts b/src/skills-placement/index.ts index b203ac6..6b78aeb 100644 --- a/src/skills-placement/index.ts +++ b/src/skills-placement/index.ts @@ -18,17 +18,19 @@ * works without it, falling back to `inline` delivery. */ -import type { Harness } from '../harness/index' +import { KNOWN_HARNESSES, type Harness } from '../harness/index' import { skillDirForHarness, type HarnessId } from '@tangle-network/agent-profile-materialize' -import { composeSkills, type ComposedSkills, type SkillEntry } from '../skills/index' +import { composeSkills, renderInlineSkills, type ComposedSkills, type SkillEntry } from '../skills/index' /** agent-app `Harness` -> platform `HarnessId`, identity-mapped for exactly * the harnesses the platform map covers. Harnesses absent here (`amp`, * `factory-droids`, `forge`, `acp`, `cursor`, `cli-base`) resolve to `null` — - * callers fall back to `inline` delivery. `cursor`'s adapter supports - * `resources.skills` bespokely but isn't in the platform map yet; treating it - * as unbridged (inline fallback) is the safe posture until the map covers - * it, rather than guessing its cwd skill dir here. */ + * {@link composeSkillsForHarness} refuses by default rather than silently + * delivering `inline`; a caller that genuinely needs `inline` on one of + * these opts in explicitly (`onNoSkillDir: 'inline'`). `cursor`'s adapter + * supports `resources.skills` bespokely but isn't in the platform map yet; + * treating it as unbridged is the safe posture until the map covers it, + * rather than guessing its cwd skill dir here. */ const HARNESS_BRIDGE: Partial> = { opencode: 'opencode', 'claude-code': 'claude-code', @@ -72,20 +74,74 @@ export interface ComposeSkillsForHarnessInput { harness: Harness tier?: string heading?: string + /** + * What to do when `harness` has no native skill-mount directory (see + * {@link resolveSkillDir}). Default `'throw'`: refuse rather than silently + * inlining every skill BODY into the system prompt. Pass `'inline'` only as + * a deliberate, auditable opt-in — the caller has decided every skill must + * reach this harness even though it will be concatenated into the prompt. + */ + onNoSkillDir?: 'throw' | 'inline' +} + +/** `KNOWN_HARNESSES` filtered to those the platform materializer can mount + * skill files onto natively (a non-null {@link resolveSkillDir}). Computed + * fresh rather than cached — the platform map can grow without a release + * here. Used only to build the actionable error in + * {@link composeSkillsForHarness}; not exported because it duplicates + * {@link unsupportedSkillHarnesses}'s complement and callers should reach + * for that instead when they need the harness list at runtime. */ +function nativeSkillMountHarnesses(): Harness[] { + return KNOWN_HARNESSES.filter((harness) => resolveSkillDir(harness) !== null) } /** - * Compose {@link ComposedSkills} for `harness`: `mounted` delivery when the - * platform names a cwd skill dir for it, `inline` delivery (the automatic - * fallback that keeps every skill available on every harness) otherwise. The - * one function a product calls instead of hand-checking `resolveSkillDir` + * Compose {@link ComposedSkills} for `harness`. + * + * `mounted` delivery is the standard path: whenever the platform names a cwd + * skill dir for `harness`, skills ride the typed `resources.skills` channel + * and the platform materializer writes each one to that directory as a real + * file — the agent reads it on demand with its own file tools. + * + * `inline` delivery — every skill BODY rendered straight into the system + * prompt — is a NARROW fallback for harnesses with no native skill dir, never + * a co-equal mode. An `AgentProfile` is a RECIPE that materializes into a + * sandbox, not a prompt template: skill bodies belong in a file mount, and + * `prompt.systemPrompt` should carry only what the agent must obey without a + * tool call (identity, tool-call contract, safety rules, output format) plus + * a short index of what's mounted. Concatenating skill bodies into the prompt + * degrades the model toward empty answers once it grows large enough — one + * product reproduced a 151,882-byte prompt exactly this way, from an + * unnoticed inline fallback. + * + * Because of that, `onNoSkillDir` defaults to `'throw'`: a harness with no + * skill dir refuses instead of silently inlining. The thrown error names the + * harness, the skill count, the byte size that would have been inlined, and + * the harnesses that DO support native mounting. Pass `onNoSkillDir: + * 'inline'` only as a deliberate, auditable opt-in when every skill genuinely + * must reach this harness regardless of prompt cost. + * + * The one function a product calls instead of hand-checking `resolveSkillDir` * and branching between {@link composeSkills}'s two modes itself. */ export function composeSkillsForHarness(input: ComposeSkillsForHarnessInput): ComposedSkills { - const { skills, harness, tier, heading } = input + const { skills, harness, tier, heading, onNoSkillDir = 'throw' } = input const skillDir = resolveSkillDir(harness) if (skillDir) return composeSkills({ skills, mode: 'mounted', skillDir, tier, heading }) - return composeSkills({ skills, mode: 'inline', tier, heading }) + if (onNoSkillDir === 'inline') return composeSkills({ skills, mode: 'inline', tier, heading }) + + const relevant = tier ? skills.filter((s) => s.tier === tier) : skills + const promptSection = renderInlineSkills({ skills, tier, heading }) + const bytes = new TextEncoder().encode(promptSection).byteLength + const mountable = nativeSkillMountHarnesses() + throw new Error( + `composeSkillsForHarness: "${harness}" has no native skill-mount directory. ` + + `Inlining ${relevant.length} skill(s) (${bytes} bytes) into its system prompt would silently ` + + `convert mounted skills into prompt text — a 151,882-byte prompt shipped this way once, and ` + + `oversized prompts degrade toward empty answers. Mount the skills instead: pick a harness with ` + + `native skill support (${mountable.join(', ')}), or pass onNoSkillDir: 'inline' to opt in ` + + `explicitly and accept the inlined bytes.`, + ) } export type { ComposedSkills, SkillEntry } from '../skills/index' diff --git a/src/skills/index.ts b/src/skills/index.ts index dcda5cd..ef98943 100644 --- a/src/skills/index.ts +++ b/src/skills/index.ts @@ -637,7 +637,8 @@ export interface ComposeSkillsInput { * directory); a null/absent one throws rather than silently falling back, so * the caller resolves the fallback deliberately (see * `@tangle-network/agent-app/skills-placement`'s `composeSkillsForHarness`, - * which does exactly that). + * whose own default is to throw rather than choose `inline` for you — inline + * is a narrow, explicitly opted-into fallback, not a co-equal mode). */ export function composeSkills(input: ComposeSkillsInput): ComposedSkills { const { skills, mode, tier, heading } = input