Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions api/app/clients/BaseClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,23 @@ class BaseClient {
userMessage.manualSkills = skills;
}
}
/**
* Persist the names of skills auto-primed this turn via `always-apply`
* frontmatter so `ManualSkillPills` can render pinned-variant badges
* on the user bubble that survive reload and history render. Frozen
* at turn time (not reconstructed from `Skill.alwaysApply` at render
* time) because the flag is mutable β€” historical turns must keep
* their audit trail even if an admin flips `alwaysApply` off later.
*/
const alwaysApplySkillPrimes = this.options.agent?.alwaysApplySkillPrimes;
if (Array.isArray(alwaysApplySkillPrimes) && alwaysApplySkillPrimes.length > 0) {
const names = alwaysApplySkillPrimes
.map((p) => p?.name)
.filter((n) => typeof n === 'string' && n.length > 0);
if (names.length > 0) {
userMessage.alwaysAppliedSkills = names;
}
}
userMessagePromise = this.saveMessageToDatabase(userMessage, saveOptions, user).catch(
(err) => {
logger.error('[BaseClient] Failed to save user message:', err);
Expand Down
8 changes: 8 additions & 0 deletions api/server/controllers/agents/__tests__/openai.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,14 @@ jest.mock('@librechat/api', () => ({
getTransactionsConfig: mockGetTransactionsConfig,
recordCollectedUsage: mockRecordCollectedUsage,
extractManualSkills: jest.fn().mockReturnValue(undefined),
injectSkillPrimes: jest.fn().mockReturnValue({
initialMessages: [],
indexTokenCountMap: {},
inserted: 0,
insertIdx: -1,
alwaysApplyDropped: 0,
alwaysApplyDedupedFromManual: 0,
}),
buildNonStreamingResponse: jest.fn().mockReturnValue({ id: 'resp-123' }),
createOpenAIStreamTracker: jest.fn().mockReturnValue({
addText: jest.fn(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,14 @@ jest.mock('@librechat/api', () => ({
getTransactionsConfig: mockGetTransactionsConfig,
recordCollectedUsage: mockRecordCollectedUsage,
extractManualSkills: jest.fn().mockReturnValue(undefined),
injectSkillPrimes: jest.fn().mockReturnValue({
initialMessages: [],
indexTokenCountMap: {},
inserted: 0,
insertIdx: -1,
alwaysApplyDropped: 0,
alwaysApplyDedupedFromManual: 0,
}),
createToolExecuteHandler: jest.fn().mockReturnValue({ handle: jest.fn() }),
// Responses API
writeDone: jest.fn(),
Expand Down
75 changes: 51 additions & 24 deletions api/server/controllers/agents/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const {
filterMalformedContentParts,
countFormattedMessageTokens,
hydrateMissingIndexTokenCounts,
injectManualSkillPrimes,
injectSkillPrimes,
isSkillPrimeMessage,
buildSkillPrimeContentParts,
} = require('@librechat/api');
Expand Down Expand Up @@ -773,26 +773,41 @@ class AgentClient extends BaseClient {
}

/**
* Phase 3 manual skill priming β€” injected by user via `$` popover.
* Skill priming β€” both manual ($ popover) and always-apply (frontmatter).
*
* Splice + index-shift logic lives in `injectManualSkillPrimes`
* Splice + index-shift logic lives in `injectSkillPrimes`
* (packages/api/src/agents/skills.ts) so the delicate position math
* can be unit-tested in TS without standing up AgentClient. Runs for
* both single-agent and multi-agent runs; how primes interact with
* handoff / added-convo agents' per-agent state is an agents-SDK
* concern, not this layer's to gate.
* can be unit-tested in TS without standing up AgentClient. The
* resolver enforces a combined ceiling (manual-first, always-apply
* truncated first when over cap) before reaching here; the splice
* re-applies the cap as defense-in-depth. Runs for both single-
* agent and multi-agent runs; how primes interact with handoff /
* added-convo agents' per-agent state is an agents-SDK concern,
* not this layer's to gate.
*/
const manualSkillPrimes = this.options.agent?.manualSkillPrimes;
if (manualSkillPrimes && manualSkillPrimes.length > 0) {
const primeResult = injectManualSkillPrimes({
const alwaysApplySkillPrimes = this.options.agent?.alwaysApplySkillPrimes;
if (
(manualSkillPrimes && manualSkillPrimes.length > 0) ||
(alwaysApplySkillPrimes && alwaysApplySkillPrimes.length > 0)
) {
Comment on lines +789 to +793
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Include always-applied IDs in primed-skill lookup map

This block now injects alwaysApplySkillPrimes, but tool execution still builds manualSkillPrimedIdsByName from manual primes only (via buildManualSkillPrimedIdsByName(...) in the agent init/service path). As a result, read_file does not treat always-applied skills as β€œprimed this turn”: always-applied skills with disable-model-invocation: true get blocked from reading their own files, and same-name collisions can resolve files from a different skill document than the one whose body was primed.

Useful? React with πŸ‘Β / πŸ‘Ž.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in fe078cc. Valid finding β€” the read_file handler was only receiving manual primes in skillPrimedIdsByName, so always-apply skills with disable-model-invocation: true got blocked reading their own bundled files and same-name collisions could shadow the primed doc. Renamed buildManualSkillPrimedIdsByName β†’ buildSkillPrimedIdsByName (accepts both manual + always-apply arrays), renamed the configurable field to skillPrimedIdsByName throughout, and expanded the gate-relaxation doc and tests. Manual wins on the rare overlap case. Added two new tests: gate-relaxation fires for always-apply, and _id pinning works for always-apply same-name collisions.

const primeResult = injectSkillPrimes({
initialMessages,
indexTokenCountMap,
manualSkillPrimes,
alwaysApplySkillPrimes,
});
indexTokenCountMap = primeResult.indexTokenCountMap;
if (primeResult.inserted > 0) {
const manualNames = (manualSkillPrimes ?? []).map((p) => p.name);
const alwaysApplyNames = (alwaysApplySkillPrimes ?? []).map((p) => p.name);
logger.debug(
`[AgentClient] Primed ${primeResult.inserted} manual skill(s) at message index ${primeResult.insertIdx}: ${manualSkillPrimes.map((p) => p.name).join(', ')}`,
`[AgentClient] Primed ${primeResult.inserted} skill(s) at message index ${primeResult.insertIdx} β€” manual: [${manualNames.join(', ')}], always-apply: [${alwaysApplyNames.join(', ')}]`,
);
}
if (primeResult.alwaysApplyDropped > 0) {
logger.warn(
`[AgentClient] Dropped ${primeResult.alwaysApplyDropped} always-apply prime(s) to stay within MAX_PRIMED_SKILLS_PER_TURN.`,
);
}
}
Expand Down Expand Up @@ -914,28 +929,40 @@ class AgentClient extends BaseClient {
await runAgents(initialMessages);

/**
* Surface a completed `skill` tool_call content part per manually-
* invoked skill so the existing `SkillCall` frontend renderer shows
* Surface a completed `skill` tool_call content part per *manually*-
* primed skill so the existing `SkillCall` frontend renderer shows
* a "Skill X loaded" card on the assistant response. Applied after
* the graph finishes to avoid clashing with the aggregator's own
* per-step content indexing. Prepended (not appended) so cards sit
* above the model's output β€” priming ran before the turn, the
* reply follows.
*
* Live streaming display of cards is handled on the user side via
* `ManualSkillPills` reading the message's `manualSkills` field;
* no separate SSE emit is needed here, and 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 below
* the reply on finalize. Post-run unshift keeps the final
* Always-apply primes intentionally do NOT emit assistant-side
* cards. `extractInvokedSkillsFromPayload` scans history for
* `skill` tool_calls and feeds `primeInvokedSkills`, which is
* Phase 3's sticky-re-prime path β€” that's the right behavior for
* manual (user picked `$skill` once; re-prime on every subsequent
* turn from history). For always-apply, `resolveAlwaysApplySkills`
* already re-primes every turn from fresh DB state, so persisting
* 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
* 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
* 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
* below the reply on finalize. Post-run unshift keeps the final
* responseMessage.content in the right order.
*/
const primedSkills = this.options.agent?.manualSkillPrimes;
if (primedSkills && primedSkills.length > 0) {
const primeParts = buildSkillPrimeContentParts(primedSkills, {
runId: this.responseMessageId ?? 'manual-skill',
});
this.contentParts.unshift(...primeParts);
const manualPrimed = this.options.agent?.manualSkillPrimes ?? [];
if (manualPrimed.length > 0) {
const runId = this.responseMessageId ?? 'skill-prime';
const manualParts = buildSkillPrimeContentParts(manualPrimed, { runId });
this.contentParts.unshift(...manualParts);
}

/** @deprecated Agent Chain */
Expand Down
28 changes: 19 additions & 9 deletions api/server/controllers/agents/openai.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const {
buildNonStreamingResponse,
createOpenAIStreamTracker,
createOpenAIContentAggregator,
injectManualSkillPrimes,
injectSkillPrimes,
isChatCompletionValidationFailure,
} = require('@librechat/api');
const {
Expand All @@ -42,7 +42,7 @@ const { findAccessibleResources } = require('~/server/services/PermissionService
const {
getSkillToolDeps,
enrichWithSkillConfigurable,
buildManualSkillPrimedIdsByName,
buildSkillPrimedIdsByName,
} = require('~/server/services/Endpoints/agents/skillDeps');
const db = require('~/models');

Expand Down Expand Up @@ -274,6 +274,7 @@ const OpenAIChatCompletionController = async (req, res) => {
getToolFilesByIds: db.getToolFilesByIds,
getCodeGeneratedFiles: db.getCodeGeneratedFiles,
listSkillsByAccess: db.listSkillsByAccess,
listAlwaysApplySkills: db.listAlwaysApplySkills,
getSkillByName: db.getSkillByName,
},
);
Expand Down Expand Up @@ -332,7 +333,10 @@ const OpenAIChatCompletionController = async (req, res) => {
req,
primaryConfig.accessibleSkillIds,
undefined,
buildManualSkillPrimedIdsByName(primaryConfig.manualSkillPrimes),
buildSkillPrimedIdsByName(
primaryConfig.manualSkillPrimes,
primaryConfig.alwaysApplySkillPrimes,
),
);
},
toolEndCallback,
Expand All @@ -350,17 +354,23 @@ const OpenAIChatCompletionController = async (req, res) => {
let indexTokenCountMap = formatted.indexTokenCountMap;

/**
* Inject manual skill primes so the model sees SKILL.md bodies for this
* turn β€” parity with AgentClient's chat path. OpenAI-compatible streaming
* uses its own tracker/aggregator shape, so the LibreChat-style card SSE
* events don't apply here; only the message-context part carries over.
* Inject manual + always-apply skill primes so the model sees SKILL.md
* bodies for this turn β€” parity with AgentClient's chat path. OpenAI-
* compatible streaming uses its own tracker/aggregator shape, so the
* LibreChat-style card SSE events don't apply here; only the
* message-context part carries over.
*/
const manualSkillPrimes = primaryConfig.manualSkillPrimes;
if (manualSkillPrimes && manualSkillPrimes.length > 0) {
const primeResult = injectManualSkillPrimes({
const alwaysApplySkillPrimes = primaryConfig.alwaysApplySkillPrimes;
if (
(manualSkillPrimes && manualSkillPrimes.length > 0) ||
(alwaysApplySkillPrimes && alwaysApplySkillPrimes.length > 0)
) {
const primeResult = injectSkillPrimes({
initialMessages: formattedMessages,
indexTokenCountMap,
manualSkillPrimes,
alwaysApplySkillPrimes,
});
indexTokenCountMap = primeResult.indexTokenCountMap;
}
Expand Down
33 changes: 23 additions & 10 deletions api/server/controllers/agents/responses.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const {
recordCollectedUsage,
getTransactionsConfig,
extractManualSkills,
injectManualSkillPrimes,
injectSkillPrimes,
createToolExecuteHandler,
// Responses API
writeDone,
Expand Down Expand Up @@ -51,7 +51,7 @@ const { findAccessibleResources } = require('~/server/services/PermissionService
const {
getSkillToolDeps,
enrichWithSkillConfigurable,
buildManualSkillPrimedIdsByName,
buildSkillPrimedIdsByName,
} = require('~/server/services/Endpoints/agents/skillDeps');
const db = require('~/models');

Expand Down Expand Up @@ -414,6 +414,7 @@ const createResponse = async (req, res) => {
getToolFilesByIds: db.getToolFilesByIds,
getCodeGeneratedFiles: db.getCodeGeneratedFiles,
listSkillsByAccess: db.listSkillsByAccess,
listAlwaysApplySkills: db.listAlwaysApplySkills,
getSkillByName: db.getSkillByName,
},
);
Expand Down Expand Up @@ -444,17 +445,23 @@ const createResponse = async (req, res) => {
let indexTokenCountMap = formatted.indexTokenCountMap;

/**
* Inject manual skill primes so the model sees SKILL.md bodies for this
* turn β€” parity with AgentClient's chat path. The Responses API uses its
* own response-builder shape, so LibreChat-style card SSE events don't
* apply; only the message-context part carries over.
* Inject manual + always-apply skill primes so the model sees SKILL.md
* bodies for this turn β€” parity with AgentClient's chat path. The
* Responses API uses its own response-builder shape, so LibreChat-
* style card SSE events don't apply; only the message-context part
* carries over.
*/
const manualSkillPrimes = primaryConfig.manualSkillPrimes;
if (manualSkillPrimes && manualSkillPrimes.length > 0) {
const primeResult = injectManualSkillPrimes({
const alwaysApplySkillPrimes = primaryConfig.alwaysApplySkillPrimes;
if (
(manualSkillPrimes && manualSkillPrimes.length > 0) ||
(alwaysApplySkillPrimes && alwaysApplySkillPrimes.length > 0)
) {
const primeResult = injectSkillPrimes({
initialMessages: formattedMessages,
indexTokenCountMap,
manualSkillPrimes,
alwaysApplySkillPrimes,
});
indexTokenCountMap = primeResult.indexTokenCountMap;
}
Expand Down Expand Up @@ -515,7 +522,10 @@ const createResponse = async (req, res) => {
req,
primaryConfig.accessibleSkillIds,
undefined,
buildManualSkillPrimedIdsByName(primaryConfig.manualSkillPrimes),
buildSkillPrimedIdsByName(
primaryConfig.manualSkillPrimes,
primaryConfig.alwaysApplySkillPrimes,
),
);
},
toolEndCallback,
Expand Down Expand Up @@ -688,7 +698,10 @@ const createResponse = async (req, res) => {
req,
primaryConfig.accessibleSkillIds,
undefined,
buildManualSkillPrimedIdsByName(primaryConfig.manualSkillPrimes),
buildSkillPrimedIdsByName(
primaryConfig.manualSkillPrimes,
primaryConfig.alwaysApplySkillPrimes,
),
);
},
toolEndCallback,
Expand Down
21 changes: 13 additions & 8 deletions api/server/services/Endpoints/agents/initialize.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const { filterFilesByAgentAccess } = require('~/server/services/Files/permission
const {
getSkillToolDeps,
enrichWithSkillConfigurable,
buildManualSkillPrimedIdsByName,
buildSkillPrimedIdsByName,
} = require('./skillDeps');
const { getModelsConfig } = require('~/server/controllers/ModelController');
const { checkPermission, findAccessibleResources } = require('~/server/services/PermissionService');
Expand Down Expand Up @@ -194,7 +194,7 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => {
req,
ctx.accessibleSkillIds,
codeApiKey,
ctx.manualSkillPrimedIdsByName,
ctx.skillPrimedIdsByName,
);
},
toolEndCallback,
Expand Down Expand Up @@ -291,19 +291,23 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => {
getCodeGeneratedFiles: db.getCodeGeneratedFiles,
filterFilesByAgentAccess,
listSkillsByAccess: db.listSkillsByAccess,
listAlwaysApplySkills: db.listAlwaysApplySkills,
getSkillByName: db.getSkillByName,
},
);

logger.debug(
`[initializeClient] Storing tool context for ${primaryConfig.id}: ${primaryConfig.toolDefinitions?.length ?? 0} tools, registry size: ${primaryConfig.toolRegistry?.size ?? '0'}`,
);
/** Maps each manually-primed skill name to the `_id` of the exact doc
* that was primed. Plumbed to `enrichWithSkillConfigurable` so the
* read_file handler can pin same-name collision lookups to the
* resolver's chosen doc. */
const manualSkillPrimedIdsByName = buildManualSkillPrimedIdsByName(
/** Maps each primed skill name (manual `$` or always-apply) to the
* `_id` of the exact doc that was primed. Plumbed to
* `enrichWithSkillConfigurable` so the read_file handler can pin
* same-name collision lookups to the resolver's chosen doc AND relax
* the disable-model-invocation gate for skills whose body is already
* in this turn's context. */
const skillPrimedIdsByName = buildSkillPrimedIdsByName(
primaryConfig.manualSkillPrimes,
primaryConfig.alwaysApplySkillPrimes,
);
agentToolContexts.set(primaryConfig.id, {
agent: primaryAgent,
Expand All @@ -312,7 +316,7 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => {
tool_resources: primaryConfig.tool_resources,
actionsEnabled: primaryConfig.actionsEnabled,
accessibleSkillIds: primaryConfig.accessibleSkillIds,
manualSkillPrimedIdsByName,
skillPrimedIdsByName,
});

const agent_ids = primaryConfig.agent_ids;
Expand Down Expand Up @@ -389,6 +393,7 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => {
getCodeGeneratedFiles: db.getCodeGeneratedFiles,
filterFilesByAgentAccess,
listSkillsByAccess: db.listSkillsByAccess,
listAlwaysApplySkills: db.listAlwaysApplySkills,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Propagate primed-skill IDs into handoff agent tool context

In the handoff-agent initialization path, initializeAgent is now given listAlwaysApplySkills, so secondary agents can resolve always-apply primes, but agentToolContexts.set(...) for those agents still omits skillPrimedIdsByName. handleReadFileCall relies on that map to pin same-name lookups and to relax disable-model-invocation for primed skills, so handoff agents with always-applied skills can fail read_file (or resolve the wrong duplicate) even though the skill was primed for the turn.

Useful? React with πŸ‘Β / πŸ‘Ž.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 06b7aed. Valid finding β€” handoff agents go through the same initializeAgent flow as the primary (now with listAlwaysApplySkills plumbed), so they resolve their own manual + always-apply primes, but the handoff-agent branch of agentToolContexts.set was dropping skillPrimedIdsByName from the per-agent context. That meant handleReadFileCall fell back to the full ACL set + prefer* flag, which could resolve same-name collisions to the wrong doc or block disable-model-invocation: true primed skills from reading their own bundled files. Now built via buildSkillPrimedIdsByName(config.manualSkillPrimes, config.alwaysApplySkillPrimes) for every handoff tool context so read_file behaves identically across primary + handoff.

getSkillByName: db.getSkillByName,
},
);
Expand Down
Loading
Loading