Skip to content

Commit 41158f7

Browse files
danny-avilafuuuzzy
authored andcommitted
πŸ“ feat: always-apply frontmatter: auto-prime skills every turn (danny-avila#12746)
* πŸ” refactor: Rebase always-apply work onto merged structured-frontmatter columns Phase 6 (disable-model-invocation / user-invocable / allowed-tools) landed first on feat/agent-skills. Reconcile this branch with the new mainline: - Thread alwaysApplySkillPrimes through unionPrimeAllowedTools alongside manualSkillPrimes, applying the combined MAX_PRIMED_SKILLS_PER_TURN ceiling before loading tools. - Add `_id` to ResolvedAlwaysApplySkill to match Phase 6's ResolvedManualSkill shape (read_file name-collision protection). - Register 'always-apply' in ALLOWED_FRONTMATTER_KEYS / FRONTMATTER_KIND so Phase 6's validator recognizes it. - Drop frontmatter from the listSkillsByAccess projection; the backfill helper remains as defensive code but its read path is no longer exercised on summary rows (no legacy rows exist β€” the branch never shipped), saving ~200KB per page. - Retire the corresponding "backfills legacy on summaries" test. - Plumb listAlwaysApplySkills through the JS controllers + endpoint initializer so the always-apply resolver sees a real DB method. * 🧹 fix: Dedupe manual/always-apply overlap, share YAML util, tidy comments Addresses review findings: - Cross-list dedup: when a user $-invokes a skill that is also marked always-apply, the always-apply copy is now dropped so the same SKILL.md body never primes twice in one turn. Manual wins (explicit intent, closer to the user message). Dedup runs in both initializeAgent (so persisted user-bubble pills stay in sync) and injectSkillPrimes (defense-in-depth at splice time). New test cases cover single-overlap, partial-overlap, and dedup-before-cap. - DRY: extract stripYamlTrailingComment to packages/data-schemas/src/utils/yaml.ts; packages/api/src/skills/import.ts now imports the shared helper. Also drop the redundant inner stripYamlTrailingComment call inside parseBooleanScalar β€” the call site already strips. - Mark injectManualSkillPrimes as @deprecated in favor of injectSkillPrimes (kept for external consumers of @librechat/api). - Document SKILL_TRIGGER_MODEL as forward-looking plumbing for the model-invoked path rather than leaving it as a bare unused export. - Replace the stale "frontmatter is included" comment on listSkillsByAccess with an accurate explanation of why it was intentionally excluded. * πŸ”’ fix: Include always-apply primes in skillPrimedIdsByName + clear alwaysApply on body opt-out Two bugs flagged by Codex review: P1 (read_file): `manualSkillPrimedIdsByName` only carried manual-invocation primes, so an always-apply skill with `disable-model-invocation: true` was blocked from reading its own bundled files, and same-name collisions could resolve to a different doc than the one whose body got primed. - Rename `buildManualSkillPrimedIdsByName` β†’ `buildSkillPrimedIdsByName` (accepts both manual + always-apply prime arrays). - Rename the configurable field `manualSkillPrimedIdsByName` β†’ `skillPrimedIdsByName` throughout the plumbing (skillConfigurable.ts, handlers.ts, CJS callers, tests). - Overlap resolution: manual wins on the rare edge case where the same name appears in both arrays (upstream dedup should prevent this, but defensive merging treats manual as authoritative). - New tests: (1) gate-relaxation fires for always-apply primes, (2) `_id` pinning works for always-apply same-name collisions. P2 (updateSkill): when a body update had no `always-apply:` key, `extractAlwaysApplyFromBody` returned `absent` and the column was left untouched. A skill that was once `alwaysApply: true` would keep auto-priming even after its SKILL.md no longer declared the flag. - Treat `absent` as a positive "not always-apply" declaration when the body is explicitly submitted; flip the column to `false`. - Explicit top-level `alwaysApply` still wins (three-source precedence unchanged). - New tests: body removes key β†’ false, body has no frontmatter at all β†’ false, explicit + body-without-key β†’ explicit wins. * 🧡 refactor: Collapse duplicate prime types + tighten parse + test hygiene Sanity-check review follow-ups: - Collapse `ResolvedManualSkill` / `ResolvedAlwaysApplySkill` into a single `ResolvedSkillPrime` canonical interface with two backward- compatible type aliases. Both resolvers feed the same pipeline stages (injectSkillPrimes, unionPrimeAllowedTools, buildSkillPrimedIdsByName); the per-source distinction lives on `additional_kwargs.trigger`, not on the resolver output. - Move the `always-apply` branch in `parseFrontmatter` to operate on the raw post-colon text. The outer `unquoteYaml` was fine today because it's idempotent on non-quoted strings, but running it twice (once per line, once after stripping the inline comment) would be fragile if the unquoter ever grows richer YAML-escape handling. - Add the missing `alwaysApplyDedupedFromManual: 0` field to the `injectSkillPrimes` mocks in `openai.spec.js` and `responses.unit.spec.js` so they match the full `InjectSkillPrimesResult` contract. - Insert the blank line between the `unionPrimeAllowedTools` and `resolveAlwaysApplySkills` describe blocks. * πŸ”§ fix(tsc): Cast mock.calls via `unknown` for strict tuple destructure `getSkillByName.mock.calls[0]` is typed as `[]` by jest's generic default; a direct cast to `[string, ..., ...]` fails TS2352 under `--noEmit` even though the runtime shape matches. Go through `as unknown as [...]` like the earlier test in the same file so CI's type-check step stays green. * πŸͺ’ fix: Propagate skillPrimedIdsByName into handoff agent tool context Handoff agents go through the same `initializeAgent` flow as the primary (with `listAlwaysApplySkills` now plumbed), so they resolve their own `manualSkillPrimes` and `alwaysApplySkillPrimes` β€” but the `agentToolContexts.set(...)` for handoff agents didn't carry `skillPrimedIdsByName` into the per-agent context. That meant `handleReadFileCall` fell back to the full ACL set + a `prefer*` flag for handoff agents: same-name collisions could resolve to a different doc than the one whose body got primed, and a `disable-model-invocation: true` skill primed via manual `$` or always-apply inside the handoff flow would be blocked from reading its own bundled files. Build the map via `buildSkillPrimedIdsByName(config.manualSkillPrimes, config.alwaysApplySkillPrimes)` for every handoff tool context so `read_file` behaves identically across primary and handoff agents.
1 parent ef079fa commit 41158f7

34 files changed

Lines changed: 2359 additions & 211 deletions

File tree

β€Žapi/app/clients/BaseClient.jsβ€Ž

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -508,6 +508,23 @@ class BaseClient {
508508
userMessage.manualSkills = skills;
509509
}
510510
}
511+
/**
512+
* Persist the names of skills auto-primed this turn via `always-apply`
513+
* frontmatter so `ManualSkillPills` can render pinned-variant badges
514+
* on the user bubble that survive reload and history render. Frozen
515+
* at turn time (not reconstructed from `Skill.alwaysApply` at render
516+
* time) because the flag is mutable β€” historical turns must keep
517+
* their audit trail even if an admin flips `alwaysApply` off later.
518+
*/
519+
const alwaysApplySkillPrimes = this.options.agent?.alwaysApplySkillPrimes;
520+
if (Array.isArray(alwaysApplySkillPrimes) && alwaysApplySkillPrimes.length > 0) {
521+
const names = alwaysApplySkillPrimes
522+
.map((p) => p?.name)
523+
.filter((n) => typeof n === 'string' && n.length > 0);
524+
if (names.length > 0) {
525+
userMessage.alwaysAppliedSkills = names;
526+
}
527+
}
511528
userMessagePromise = this.saveMessageToDatabase(userMessage, saveOptions, user).catch(
512529
(err) => {
513530
logger.error('[BaseClient] Failed to save user message:', err);

β€Žapi/server/controllers/agents/__tests__/openai.spec.jsβ€Ž

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,14 @@ jest.mock('@librechat/api', () => ({
5959
getTransactionsConfig: mockGetTransactionsConfig,
6060
recordCollectedUsage: mockRecordCollectedUsage,
6161
extractManualSkills: jest.fn().mockReturnValue(undefined),
62+
injectSkillPrimes: jest.fn().mockReturnValue({
63+
initialMessages: [],
64+
indexTokenCountMap: {},
65+
inserted: 0,
66+
insertIdx: -1,
67+
alwaysApplyDropped: 0,
68+
alwaysApplyDedupedFromManual: 0,
69+
}),
6270
buildNonStreamingResponse: jest.fn().mockReturnValue({ id: 'resp-123' }),
6371
createOpenAIStreamTracker: jest.fn().mockReturnValue({
6472
addText: jest.fn(),

β€Žapi/server/controllers/agents/__tests__/responses.unit.spec.jsβ€Ž

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,14 @@ jest.mock('@librechat/api', () => ({
6161
getTransactionsConfig: mockGetTransactionsConfig,
6262
recordCollectedUsage: mockRecordCollectedUsage,
6363
extractManualSkills: jest.fn().mockReturnValue(undefined),
64+
injectSkillPrimes: jest.fn().mockReturnValue({
65+
initialMessages: [],
66+
indexTokenCountMap: {},
67+
inserted: 0,
68+
insertIdx: -1,
69+
alwaysApplyDropped: 0,
70+
alwaysApplyDedupedFromManual: 0,
71+
}),
6472
createToolExecuteHandler: jest.fn().mockReturnValue({ handle: jest.fn() }),
6573
// Responses API
6674
writeDone: jest.fn(),

β€Žapi/server/controllers/agents/client.jsβ€Ž

Lines changed: 51 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ const {
2828
filterMalformedContentParts,
2929
countFormattedMessageTokens,
3030
hydrateMissingIndexTokenCounts,
31-
injectManualSkillPrimes,
31+
injectSkillPrimes,
3232
isSkillPrimeMessage,
3333
buildSkillPrimeContentParts,
3434
} = require('@librechat/api');
@@ -773,26 +773,41 @@ class AgentClient extends BaseClient {
773773
}
774774

775775
/**
776-
* Phase 3 manual skill priming β€” injected by user via `$` popover.
776+
* Skill priming β€” both manual ($ popover) and always-apply (frontmatter).
777777
*
778-
* Splice + index-shift logic lives in `injectManualSkillPrimes`
778+
* Splice + index-shift logic lives in `injectSkillPrimes`
779779
* (packages/api/src/agents/skills.ts) so the delicate position math
780-
* can be unit-tested in TS without standing up AgentClient. Runs for
781-
* both single-agent and multi-agent runs; how primes interact with
782-
* handoff / added-convo agents' per-agent state is an agents-SDK
783-
* concern, not this layer's to gate.
780+
* can be unit-tested in TS without standing up AgentClient. The
781+
* resolver enforces a combined ceiling (manual-first, always-apply
782+
* truncated first when over cap) before reaching here; the splice
783+
* re-applies the cap as defense-in-depth. Runs for both single-
784+
* agent and multi-agent runs; how primes interact with handoff /
785+
* added-convo agents' per-agent state is an agents-SDK concern,
786+
* not this layer's to gate.
784787
*/
785788
const manualSkillPrimes = this.options.agent?.manualSkillPrimes;
786-
if (manualSkillPrimes && manualSkillPrimes.length > 0) {
787-
const primeResult = injectManualSkillPrimes({
789+
const alwaysApplySkillPrimes = this.options.agent?.alwaysApplySkillPrimes;
790+
if (
791+
(manualSkillPrimes && manualSkillPrimes.length > 0) ||
792+
(alwaysApplySkillPrimes && alwaysApplySkillPrimes.length > 0)
793+
) {
794+
const primeResult = injectSkillPrimes({
788795
initialMessages,
789796
indexTokenCountMap,
790797
manualSkillPrimes,
798+
alwaysApplySkillPrimes,
791799
});
792800
indexTokenCountMap = primeResult.indexTokenCountMap;
793801
if (primeResult.inserted > 0) {
802+
const manualNames = (manualSkillPrimes ?? []).map((p) => p.name);
803+
const alwaysApplyNames = (alwaysApplySkillPrimes ?? []).map((p) => p.name);
794804
logger.debug(
795-
`[AgentClient] Primed ${primeResult.inserted} manual skill(s) at message index ${primeResult.insertIdx}: ${manualSkillPrimes.map((p) => p.name).join(', ')}`,
805+
`[AgentClient] Primed ${primeResult.inserted} skill(s) at message index ${primeResult.insertIdx} β€” manual: [${manualNames.join(', ')}], always-apply: [${alwaysApplyNames.join(', ')}]`,
806+
);
807+
}
808+
if (primeResult.alwaysApplyDropped > 0) {
809+
logger.warn(
810+
`[AgentClient] Dropped ${primeResult.alwaysApplyDropped} always-apply prime(s) to stay within MAX_PRIMED_SKILLS_PER_TURN.`,
796811
);
797812
}
798813
}
@@ -915,28 +930,40 @@ class AgentClient extends BaseClient {
915930
await runAgents(initialMessages);
916931

917932
/**
918-
* Surface a completed `skill` tool_call content part per manually-
919-
* invoked skill so the existing `SkillCall` frontend renderer shows
933+
* Surface a completed `skill` tool_call content part per *manually*-
934+
* primed skill so the existing `SkillCall` frontend renderer shows
920935
* a "Skill X loaded" card on the assistant response. Applied after
921936
* the graph finishes to avoid clashing with the aggregator's own
922937
* per-step content indexing. Prepended (not appended) so cards sit
923938
* above the model's output β€” priming ran before the turn, the
924939
* reply follows.
925940
*
926-
* Live streaming display of cards is handled on the user side via
927-
* `ManualSkillPills` reading the message's `manualSkills` field;
928-
* no separate SSE emit is needed here, and trying to stream a
929-
* mid-run tool_call at index 0 collided with the LLM's first text
930-
* content, while emitting at a sparse offset pushed the card below
931-
* the reply on finalize. Post-run unshift keeps the final
941+
* Always-apply primes intentionally do NOT emit assistant-side
942+
* cards. `extractInvokedSkillsFromPayload` scans history for
943+
* `skill` tool_calls and feeds `primeInvokedSkills`, which is
944+
* Phase 3's sticky-re-prime path β€” that's the right behavior for
945+
* manual (user picked `$skill` once; re-prime on every subsequent
946+
* turn from history). For always-apply, `resolveAlwaysApplySkills`
947+
* already re-primes every turn from fresh DB state, so persisting
948+
* the card would cause the skill body to get primed twice per
949+
* turn starting on turn 2. The user-facing acknowledgement for
950+
* always-apply lives on the user bubble as the pinned
951+
* `ManualSkillPills` row (`message.alwaysAppliedSkills`), which
952+
* is the durable signal the user wants: "this skill auto-primes".
953+
*
954+
* Live streaming display of manual user-bubble pills is handled
955+
* by `ManualSkillPills` reading `message.manualSkills`. No
956+
* separate SSE emit is needed here; trying to stream a mid-run
957+
* tool_call at index 0 collided with the LLM's first text
958+
* content, while emitting at a sparse offset pushed the card
959+
* below the reply on finalize. Post-run unshift keeps the final
932960
* responseMessage.content in the right order.
933961
*/
934-
const primedSkills = this.options.agent?.manualSkillPrimes;
935-
if (primedSkills && primedSkills.length > 0) {
936-
const primeParts = buildSkillPrimeContentParts(primedSkills, {
937-
runId: this.responseMessageId ?? 'manual-skill',
938-
});
939-
this.contentParts.unshift(...primeParts);
962+
const manualPrimed = this.options.agent?.manualSkillPrimes ?? [];
963+
if (manualPrimed.length > 0) {
964+
const runId = this.responseMessageId ?? 'skill-prime';
965+
const manualParts = buildSkillPrimeContentParts(manualPrimed, { runId });
966+
this.contentParts.unshift(...manualParts);
940967
}
941968

942969
/** @deprecated Agent Chain */

β€Žapi/server/controllers/agents/openai.jsβ€Ž

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ const {
2929
buildNonStreamingResponse,
3030
createOpenAIStreamTracker,
3131
createOpenAIContentAggregator,
32-
injectManualSkillPrimes,
32+
injectSkillPrimes,
3333
isChatCompletionValidationFailure,
3434
discoverConnectedAgents,
3535
getRemoteAgentPermissions,
@@ -48,7 +48,7 @@ const {
4848
const {
4949
getSkillToolDeps,
5050
enrichWithSkillConfigurable,
51-
buildManualSkillPrimedIdsByName,
51+
buildSkillPrimedIdsByName,
5252
} = require('~/server/services/Endpoints/agents/skillDeps');
5353
const { getModelsConfig } = require('~/server/controllers/ModelController');
5454
const { logViolation } = require('~/cache');
@@ -246,6 +246,7 @@ const OpenAIChatCompletionController = async (req, res) => {
246246
getToolFilesByIds: db.getToolFilesByIds,
247247
getCodeGeneratedFiles: db.getCodeGeneratedFiles,
248248
listSkillsByAccess: db.listSkillsByAccess,
249+
listAlwaysApplySkills: db.listAlwaysApplySkills,
249250
getSkillByName: db.getSkillByName,
250251
};
251252

@@ -432,7 +433,10 @@ const OpenAIChatCompletionController = async (req, res) => {
432433
req,
433434
primaryConfig.accessibleSkillIds,
434435
undefined,
435-
buildManualSkillPrimedIdsByName(primaryConfig.manualSkillPrimes),
436+
buildSkillPrimedIdsByName(
437+
primaryConfig.manualSkillPrimes,
438+
primaryConfig.alwaysApplySkillPrimes,
439+
),
436440
);
437441
},
438442
toolEndCallback,
@@ -450,17 +454,23 @@ const OpenAIChatCompletionController = async (req, res) => {
450454
let indexTokenCountMap = formatted.indexTokenCountMap;
451455

452456
/**
453-
* Inject manual skill primes so the model sees SKILL.md bodies for this
454-
* turn β€” parity with AgentClient's chat path. OpenAI-compatible streaming
455-
* uses its own tracker/aggregator shape, so the LibreChat-style card SSE
456-
* events don't apply here; only the message-context part carries over.
457+
* Inject manual + always-apply skill primes so the model sees SKILL.md
458+
* bodies for this turn β€” parity with AgentClient's chat path. OpenAI-
459+
* compatible streaming uses its own tracker/aggregator shape, so the
460+
* LibreChat-style card SSE events don't apply here; only the
461+
* message-context part carries over.
457462
*/
458463
const manualSkillPrimes = primaryConfig.manualSkillPrimes;
459-
if (manualSkillPrimes && manualSkillPrimes.length > 0) {
460-
const primeResult = injectManualSkillPrimes({
464+
const alwaysApplySkillPrimes = primaryConfig.alwaysApplySkillPrimes;
465+
if (
466+
(manualSkillPrimes && manualSkillPrimes.length > 0) ||
467+
(alwaysApplySkillPrimes && alwaysApplySkillPrimes.length > 0)
468+
) {
469+
const primeResult = injectSkillPrimes({
461470
initialMessages: formattedMessages,
462471
indexTokenCountMap,
463472
manualSkillPrimes,
473+
alwaysApplySkillPrimes,
464474
});
465475
indexTokenCountMap = primeResult.indexTokenCountMap;
466476
}

β€Žapi/server/controllers/agents/responses.jsβ€Ž

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ const {
2020
recordCollectedUsage,
2121
getTransactionsConfig,
2222
extractManualSkills,
23-
injectManualSkillPrimes,
23+
injectSkillPrimes,
2424
createToolExecuteHandler,
2525
discoverConnectedAgents,
2626
getRemoteAgentPermissions,
@@ -57,7 +57,7 @@ const {
5757
const {
5858
getSkillToolDeps,
5959
enrichWithSkillConfigurable,
60-
buildManualSkillPrimedIdsByName,
60+
buildSkillPrimedIdsByName,
6161
} = require('~/server/services/Endpoints/agents/skillDeps');
6262
const { getModelsConfig } = require('~/server/controllers/ModelController');
6363
const { logViolation } = require('~/cache');
@@ -373,6 +373,7 @@ const createResponse = async (req, res) => {
373373
getToolFilesByIds: db.getToolFilesByIds,
374374
getCodeGeneratedFiles: db.getCodeGeneratedFiles,
375375
listSkillsByAccess: db.listSkillsByAccess,
376+
listAlwaysApplySkills: db.listAlwaysApplySkills,
376377
getSkillByName: db.getSkillByName,
377378
};
378379

@@ -534,17 +535,23 @@ const createResponse = async (req, res) => {
534535
let indexTokenCountMap = formatted.indexTokenCountMap;
535536

536537
/**
537-
* Inject manual skill primes so the model sees SKILL.md bodies for this
538-
* turn β€” parity with AgentClient's chat path. The Responses API uses its
539-
* own response-builder shape, so LibreChat-style card SSE events don't
540-
* apply; only the message-context part carries over.
538+
* Inject manual + always-apply skill primes so the model sees SKILL.md
539+
* bodies for this turn β€” parity with AgentClient's chat path. The
540+
* Responses API uses its own response-builder shape, so LibreChat-
541+
* style card SSE events don't apply; only the message-context part
542+
* carries over.
541543
*/
542544
const manualSkillPrimes = primaryConfig.manualSkillPrimes;
543-
if (manualSkillPrimes && manualSkillPrimes.length > 0) {
544-
const primeResult = injectManualSkillPrimes({
545+
const alwaysApplySkillPrimes = primaryConfig.alwaysApplySkillPrimes;
546+
if (
547+
(manualSkillPrimes && manualSkillPrimes.length > 0) ||
548+
(alwaysApplySkillPrimes && alwaysApplySkillPrimes.length > 0)
549+
) {
550+
const primeResult = injectSkillPrimes({
545551
initialMessages: formattedMessages,
546552
indexTokenCountMap,
547553
manualSkillPrimes,
554+
alwaysApplySkillPrimes,
548555
});
549556
indexTokenCountMap = primeResult.indexTokenCountMap;
550557
}
@@ -607,7 +614,10 @@ const createResponse = async (req, res) => {
607614
req,
608615
primaryConfig.accessibleSkillIds,
609616
undefined,
610-
buildManualSkillPrimedIdsByName(primaryConfig.manualSkillPrimes),
617+
buildSkillPrimedIdsByName(
618+
primaryConfig.manualSkillPrimes,
619+
primaryConfig.alwaysApplySkillPrimes,
620+
),
611621
);
612622
},
613623
toolEndCallback,
@@ -783,7 +793,10 @@ const createResponse = async (req, res) => {
783793
req,
784794
primaryConfig.accessibleSkillIds,
785795
undefined,
786-
buildManualSkillPrimedIdsByName(primaryConfig.manualSkillPrimes),
796+
buildSkillPrimedIdsByName(
797+
primaryConfig.manualSkillPrimes,
798+
primaryConfig.alwaysApplySkillPrimes,
799+
),
787800
);
788801
},
789802
toolEndCallback,

0 commit comments

Comments
Β (0)