fix(skills-placement): require opt-in for inline skill delivery

[drewstone]

composeSkillsForHarness silently rendered every skill body into the system prompt whenever resolveSkillDir(harness) returned null — 7 of 14 known harnesses (amp, factory-droids, hermes, forge, acp, cursor, cli-base). Switching harness therefore converted mounted skills into a giant prompt with no signal; one app reproduced a 151,882-byte prompt this way.

Adds onNoSkillDir: 'throw' | 'inline', defaulting to throw. The error names the harness, the skill count, the exact UTF-8 byte size that would have been inlined (computed from the real renderInlineSkills output, not a proxy), and which harnesses do support mounting.

Verified: tsc --noEmit clean; 193 files / 3,148 tests passing (baseline 3,143 + 5 new); 23/23 in the target suite; mutation-checked — reverting to the silent fallback fails the new tests.

[tangletools]

✅ Auto-approved drewstone PR — 01a1d3ca

This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

_{tangletools · auto-approval · reason: drewstone_author · 2026-07-24T23:51:30Z}

[tangletools]

🟡 Value Audit — sound-with-nits

Verdict	sound-with-nits
Concerns	2 (2 weak-concern)
Heuristic	0.0s
Duplication	0.0s
Interrogation	103.9s (2 bridge agents)
Total	103.9s

💰 Value — sound-with-nits

Flips composeSkillsForHarness from silently inlining every skill body into the prompt (a 151KB-prompt footgun) to throwing by default with an actionable error — a clean, grain-aligned fail-loud change with an explicit opt-in escape hatch.

• What it does: Adds onNoSkillDir: 'throw' | 'inline' to composeSkillsForHarness (src/skills-placement/index.ts:127), defaulting to 'throw'. For the 7 of 14 KNOWN_HARNESSES with no native skill-mount dir (amp, factory-droids, hermes, forge, acp, cursor, cli-base), the old code silently fell back to inline delivery — rendering every skill BODY into systemPrompt. Now it throws an error naming the harness,
• Goals it achieves: Stop a silent prompt-bloat footgun: a product reproduced a 151,882-byte system prompt this way because switching harness invisibly converted mounted skills into prompt text with no signal. The doctrinal point (spelled out in the new JSDoc) is that 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, a
• Assessment: Strong change, squarely in the codebase's grain. The repo's dominant pattern is fail-loud-not-silent-fallback — a grep for silently|fail.?loud|refuse|rather than silently returns ~100 hits across crypto, billing, tools/registry, config, intakes, sandbox, missions, sequences, and the sibling parseSkillFrontmatter/composeSkills in this very module. The design is coherent: (1) default-throw mat
• Better / existing approach: none — this is the right approach. Searched for an existing fail-loud-on-no-skill-dir guard to extend (none — composeSkills requires explicit mode but the high-level composeSkillsForHarness was the only auto-fallback path, and it's the one being fixed here). Considered warn-vs-throw (warn-and-continue would be ignored, defeating the point for a silent prompt-size regression; the codebase's g
• Model: opencode/zai-coding-plan/glm-5.2
• Bridge attempts: 2
• Bridge warning: opencode/kimi-for-coding/k2p7: bridge stream ended without value-audit content

🎯 Usefulness — sound

Converts a silent 150KB+ prompt footgun (auto-inlining skill bodies for 7 of 14 harnesses) into a loud, actionable error with a one-line opt-in escape hatch — extending the codebase's existing fail-loud grain to the harness-resolution layer.

• Integration: Reachable and correctly wired. composeSkillsForHarness is the documented public entry point of the /skills-placement subpath and is named as the canonical caller by ProfileChannels.skillRefs (src/profile/index.ts:84) and by composeSkills' own JSDoc (src/skills/index.ts:639). No internal call site exists, but that is expected — agent-app is an app-shell library consumed by external products
• Fit with existing patterns: Matches the established grain exactly. composeSkills already throws when skillDir is null for mounted mode (src/skills/index.ts:654-659); this PR extends that same fail-loud posture one layer up to the function that chooses the mode. The pre-existing unsupportedSkillHarnesses (src/skills-placement/index.ts:59) already enumerates unbridged harnesses, and the new private `nativeSkillMountHar
• Real-world viability: Holds up past the happy path. The byte count in the error is computed from the REAL renderInlineSkills output (src/skills-placement/index.ts:134-135) — the identical call that composeSkills({mode:'inline'}) makes at src/skills/index.ts:649 — so the warning number is what would actually ship, not a proxy. The error names the harness, the tier-filtered skill count, the byte size, the harnesses t
• Model: opencode/zai-coding-plan/glm-5.2
• Bridge attempts: 1

💰 Value Audit

🟡 AGENTS.md module map still advertises the silent fallback this PR removes [maintenance] ``

The PR updates doc comments in src/skills-placement/index.ts, src/skills/index.ts, docs/codemap.json, docs/api/skills-placement.md, and docs/llms-full.txt, but AGENTS.md:43 — the module map agents read as the canonical reference — still lists composeSkillsForHarness (auto-inline fallback for harnesses with no cwd skill dir — hermes, amp, …) as a FEATURE. That now directly contradicts the change's intent and could mislead the next agent (or human) into expecting the old behavior. One-line edit

🎯 Usefulness Audit

🟡 Error message bakes in a one-off historical anecdote [ergonomics] ``

The thrown string hardcodes 'a 151,882-byte prompt shipped this way once' (src/skills-placement/index.ts:140). Runtime error messages should be timeless and self-justifying; this anecdote will read as stale/arbitrary in a year and is redundant — the message already shows the caller's OWN computed byte count two words earlier. Dropping the historical clause leaves the message purely actionable (harness + count + bytes + mountable list + opt-in) without dating it. Does not gate shipping.

---

What this audit checks

It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.

Pass	What it asks
Heuristic	Vague title? Whitespace-only or cruft-bearing diff? (content signals only)
Duplication	Do added function/class names already exist elsewhere in the repo?
Value Audit	What does it do? What goal does it achieve? Is it good? Better architecture or already-exists?
Usefulness Audit	Does it integrate and fit? Will it hold up in real use and actually get used?

Findings are concerns, not blocks — the human reviewer decides what to do with them.

_{value-audit · 20260724T235501Z}

[tangletools]

✅ No Blockers — 01a1d3ca

Review health 100/100 · Reviewer score 83/100 · Confidence 85/100 · 4 findings (4 low)

glm: Correctness 83 · Security 83 · Testing 83 · Architecture 83

Reviewer score is advisory once the run is complete and the verdict has no blockers.

Full multi-shot audit completed 5/5 planned shots over 6 changed files. Global verifier still owns final merge decision.

🟡 LOW Summary loses delivery-mode and onNoSkillDir detail — docs/llms-full.txt

The new one-line summary ('Compose {@link ComposedSkills} for harness.') is accurate but no longer mentions the mounted-vs-inline modes or the new onNoSkillDir opt-in that is the whole point of this commit. llms-full.txt is an auto-generated summary index, so this is consistent with how peer entries read, but a reader scanning only this file loses the actionable signal. Informational only — no correctness impact; the source docstring (src/skills-placement/index.ts:98-126) and docs/api/skills-placement.md (also updated in this commit) retain the full explanation.

🟡 LOW Default behavior change on a public export is a breaking change for existing callers — src/skills-placement/index.ts

The default for onNoSkillDir flipped from silent-inline to 'throw'. Any downstream product currently calling composeSkillsForHarness({skills, harness:'amp'}) (or hermes/forge/acp/cursor/cli-base/factory-droids) without the new option will now crash at runtime instead of silently producing a large prompt. This is the INTENDED fix (fail-loud surfaces the latent 151KB-prompt bug) and the opt-in onNoSkillDir:'inline' preserves the old behavior, so it is a justified, documented break with a clear migration. AGENTS.md invariant 5 says 'never a breaking change to an existing export' — this technically tensions that, so the global verifier should confirm the PR's version bump (currently 0.43.59 -> check CHANGELOG/semver) reflects the break, or that all known callers have been updated. No in-repo c

🟡 LOW Error message hardcodes an anecdotal byte count ('151,882-byte') that may read as a live measurement — src/skills-placement/index.ts

The thrown error string embeds 'a 151,882-byte prompt shipped this way once' as a static historical anecdote. The actual would-be-inlined size is already reported dynamically via ${bytes} on the same line, so the hardcoded figure is illustrative only — but a reader encountering this error in the wild may misread 151,882 as the current prompt's size. Stylistic nit; consider phrasing that clearly marks it as a past incident (e.g. 'a prior incident saw a 151,882-byte prompt ship this way') or dropping the specific number. No correctness impact.

🟡 LOW Throws even when zero skills survive the tier filter, producing a slightly misleading 'Inlining 0 skill(s) (0 bytes)' message — src/skills-placement/index.ts

When tier is set and no skill matches, relevant.length is 0 and renderInlineSkills returns '' (0 bytes), but the function still throws 'Inlining 0 skill(s) (0 bytes) ... would silently convert mounted skills into prompt text'. Throwing is arguably still correct (the harness genuinely has no skill dir regardless of skill count), but the message's 'would convert mounted skills into prompt text' is vacuously false when there are no skills to convert. Edge case only; harmless since a caller filtering to zero skills on an unbridged harness is already in a no-op scenario. No fix required unless the message is user-facing.

---

_{tangletools · 2026-07-24T23:58:31Z · trace}

[tangletools]

✅ Approved — 4 non-blocking findings — 01a1d3ca

Full multi-shot audit completed 5/5 planned shots over 6 changed files. Global verifier still owns final merge decision.

Full immutable report for this review: trace

Summary comment for this run: full summary

---

_{tangletools · 2026-07-24T23:58:31Z · immutable trace}