🧽 refactor: Skills polish: precedence-aware body validation, controller drop logs, SkillPills rename (#12760)

Post-merge sanity-review cleanup on top of #12746:

- `createSkill` / `updateSkill` now parse SKILL.md body's always-apply
  status once and reuse it for both validation and derivation (was
  parsing the same YAML block twice per call).
- Body-inline `always-apply:` validation becomes precedence-aware: a
  caller sending an explicit top-level `alwaysApply` or a structured
  `frontmatter['always-apply']` no longer gets rejected for a typo in
  the body — the body value is never consulted at derivation time when
  a higher-precedence source wins. New tests cover the three relevant
  interactions (explicit+body-typo, frontmatter+body-typo, body-only
  typo still rejects).
- OpenAI and Responses controllers now emit a `logger.warn` when
  `injectSkillPrimes` drops always-apply primes to stay under
  `MAX_PRIMED_SKILLS_PER_TURN`. `injectSkillPrimes` already logs
  internally; the controller-level warn adds endpoint context so
  operators can identify which path hit the cap at a glance. Mirrors
  AgentClient's existing log.
- Rename `ManualSkillPills` → `SkillPills` (component + type + file +
  test + all JSDoc references). The component handles both manual and
  always-apply pills now; the original name was carried over from the
  manual-only Phase 3 and misleads new readers.
- Drive-by fix: declare `appConfig = req.config` at the top of
  `createResponse` in `responses.js` — it was used unqualified on
  lines 381/396, which silently evaluated to `undefined` (via optional
  chaining) and disabled the skills-capability check on the Responses
  endpoint. Pre-existing, surfaced by lint on the touched file.
This commit is contained in:
Danny Avila 2026-04-21 11:07:21 -07:00
parent 7581540ab6
commit 91cd3f7b7c
17 changed files with 180 additions and 37 deletions

View file

@ -948,11 +948,11 @@ class AgentClient extends BaseClient {
* the card would cause the skill body to get primed twice per
* turn starting on turn 2. The user-facing acknowledgement for
* always-apply lives on the user bubble as the pinned
* `ManualSkillPills` row (`message.alwaysAppliedSkills`), which
* `SkillPills` row (`message.alwaysAppliedSkills`), which
* is the durable signal the user wants: "this skill auto-primes".
*
* Live streaming display of manual user-bubble pills is handled
* by `ManualSkillPills` reading `message.manualSkills`. No
* by `SkillPills` reading `message.manualSkills`. No
* separate SSE emit is needed here; trying to stream a mid-run
* tool_call at index 0 collided with the LLM's first text
* content, while emitting at a sparse offset pushed the card

View file

@ -480,6 +480,16 @@ const OpenAIChatCompletionController = async (req, res) => {
alwaysApplySkillPrimes,
});
indexTokenCountMap = primeResult.indexTokenCountMap;
/* Surface the cap-driven always-apply truncation at the controller
layer too `injectSkillPrimes` already logs internally, but the
controller-level warn includes endpoint context so operators can
tell at a glance which path hit the cap. Mirrors AgentClient's
warn in `client.js`. */
if (primeResult.alwaysApplyDropped > 0) {
logger.warn(
`[OpenAI API] Dropped ${primeResult.alwaysApplyDropped} always-apply prime(s) to stay within MAX_PRIMED_SKILLS_PER_TURN.`,
);
}
}
/**

View file

@ -378,6 +378,7 @@ const createResponse = async (req, res) => {
getSkillByName: db.getSkillByName,
};
const appConfig = req.config;
const enabledCapabilities = new Set(
appConfig?.endpoints?.[EModelEndpoint.agents]?.capabilities,
);
@ -555,6 +556,16 @@ const createResponse = async (req, res) => {
alwaysApplySkillPrimes,
});
indexTokenCountMap = primeResult.indexTokenCountMap;
/* Surface the cap-driven always-apply truncation at the controller
layer too `injectSkillPrimes` already logs internally, but the
controller-level warn includes endpoint context so operators can
tell at a glance which path hit the cap. Mirrors AgentClient's
warn in `client.js`. */
if (primeResult.alwaysApplyDropped > 0) {
logger.warn(
`[Responses API] Dropped ${primeResult.alwaysApplyDropped} always-apply prime(s) to stay within MAX_PRIMED_SKILLS_PER_TURN.`,
);
}
}
/* Stable for the turn: the capability set is the admin config, and