mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 14:57:42 +00:00
🚦 fix: Bound, Single-Flight, and Retry Skill File Priming Uploads (#14611)
Some checks are pending
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
GitNexus Index / index (push) Waiting to run
GitNexus Index / post-index (push) Blocked by required conditions
Docker Dev Images Build / build (Dockerfile, librechat-dev, node) (push) Waiting to run
Docker Dev Images Build / build (Dockerfile.multi, librechat-dev-api, api-build) (push) Waiting to run
Sync Locize Translations & Create Translation PR / Sync Translation Keys with Locize (push) Waiting to run
Sync Locize Translations & Create Translation PR / Create Translation PR on Version Published (push) Blocked by required conditions
Sync Helm Chart Tags / Ignore non-main push (push) Waiting to run
Sync Helm Chart Tags / Sync chart tags (push) Waiting to run
Some checks are pending
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
GitNexus Index / index (push) Waiting to run
GitNexus Index / post-index (push) Blocked by required conditions
Docker Dev Images Build / build (Dockerfile, librechat-dev, node) (push) Waiting to run
Docker Dev Images Build / build (Dockerfile.multi, librechat-dev-api, api-build) (push) Waiting to run
Sync Locize Translations & Create Translation PR / Sync Translation Keys with Locize (push) Waiting to run
Sync Locize Translations & Create Translation PR / Create Translation PR on Version Published (push) Blocked by required conditions
Sync Helm Chart Tags / Ignore non-main push (push) Waiting to run
Sync Helm Chart Tags / Sync chart tags (push) Waiting to run
Skill priming fanned out one unbounded batch upload per cold skill, bursting through codeapi's per-user upload limiter (30 per 5 min). Failures degraded silently: nothing persisted, every turn re-burned budget, and handle_skill reported success with no files mounted. - Bound batch uploads to 3 process-wide slots across both prime paths - Single-flight primeSkillFiles per (skill id, version) - Retry a 429 once per Retry-After, capped at 15s, fresh streams - handle_skill now tells the model when bundled files are unavailable - Warn on fulfilled-null primes in primeInvokedSkills
This commit is contained in:
parent
5029dd467e
commit
120ee2afa6
4 changed files with 457 additions and 87 deletions
|
|
@ -1,3 +1,4 @@
|
|||
import { Readable } from 'stream';
|
||||
import { Constants } from '@librechat/agents';
|
||||
import { logger } from '@librechat/data-schemas';
|
||||
import type {
|
||||
|
|
@ -553,6 +554,48 @@ describe('createToolExecuteHandler', () => {
|
|||
return createToolExecuteHandler({ loadTools, getSkillByName });
|
||||
}
|
||||
|
||||
/** Skill with one bundled file plus every dep the priming gate requires,
|
||||
* so the handler actually attempts the batch upload. */
|
||||
function createPrimingSkillHandler(
|
||||
skillName: string,
|
||||
batchUploadCodeEnvFiles: NonNullable<ToolExecuteOptions['batchUploadCodeEnvFiles']>,
|
||||
) {
|
||||
const loadTools: ToolExecuteOptions['loadTools'] = jest.fn(async () => ({
|
||||
loadedTools: [],
|
||||
configurable: {
|
||||
accessibleSkillIds: skillsInScope(),
|
||||
codeEnvAvailable: true,
|
||||
req: { user: { id: 'user-1' } },
|
||||
},
|
||||
}));
|
||||
const getSkillByName: ToolExecuteOptions['getSkillByName'] = jest.fn(async () => ({
|
||||
_id: `${skillName}-id` as unknown as never,
|
||||
name: skillName,
|
||||
body: 'skill body',
|
||||
fileCount: 1,
|
||||
version: 1,
|
||||
}));
|
||||
const listSkillFiles: ToolExecuteOptions['listSkillFiles'] = jest.fn(async () => [
|
||||
{
|
||||
relativePath: 'references/style.md',
|
||||
filename: 'style.md',
|
||||
filepath: `/storage/${skillName}/references/style.md`,
|
||||
source: 's3',
|
||||
bytes: 256,
|
||||
},
|
||||
]);
|
||||
const getStrategyFunctions: ToolExecuteOptions['getStrategyFunctions'] = jest.fn(() => ({
|
||||
getDownloadStream: jest.fn(async () => Readable.from(Buffer.from(''))),
|
||||
}));
|
||||
return createToolExecuteHandler({
|
||||
loadTools,
|
||||
getSkillByName,
|
||||
listSkillFiles,
|
||||
getStrategyFunctions,
|
||||
batchUploadCodeEnvFiles,
|
||||
});
|
||||
}
|
||||
|
||||
it('rejects with a clear error when the named skill has disableModelInvocation=true', async () => {
|
||||
const getSkillByName = jest.fn(async () => ({
|
||||
_id: 'skill-id' as unknown as never,
|
||||
|
|
@ -686,6 +729,63 @@ describe('createToolExecuteHandler', () => {
|
|||
expect(callOptions).not.toHaveProperty('preferUserInvocable', true);
|
||||
});
|
||||
|
||||
it('appends an unavailability note when file priming fails, so the model avoids dead sandbox paths', async () => {
|
||||
const batchUploadCodeEnvFiles = jest.fn(async () => {
|
||||
throw new Error('Request failed with status code 429');
|
||||
});
|
||||
const handler = createPrimingSkillHandler('note-fail-skill', batchUploadCodeEnvFiles);
|
||||
|
||||
const [result] = await invokeHandler(handler, [
|
||||
{
|
||||
id: 'call_prime_fail',
|
||||
name: Constants.SKILL_TOOL,
|
||||
args: { skillName: 'note-fail-skill' },
|
||||
},
|
||||
]);
|
||||
|
||||
/* The skill body still loads (instructions inject regardless), but the
|
||||
* tool result must say the bundled files never reached the sandbox. */
|
||||
expect(result.status).toBe('success');
|
||||
expect(result.content).toContain('could not be loaded into the code environment');
|
||||
expect(result.content).toContain('/mnt/data/skills/note-fail-skill/');
|
||||
expect(result.content).toContain('read_file');
|
||||
expect(result.artifact).toBeUndefined();
|
||||
});
|
||||
|
||||
it('omits the unavailability note when file priming succeeds', async () => {
|
||||
const batchUploadCodeEnvFiles = jest.fn(async () => ({
|
||||
storage_session_id: 'session-ok',
|
||||
files: [
|
||||
{ fileId: 'file-ok', filename: 'skills/note-ok-skill/references/style.md' },
|
||||
{ fileId: 'file-skillmd', filename: 'skills/note-ok-skill/SKILL.md' },
|
||||
],
|
||||
}));
|
||||
const handler = createPrimingSkillHandler('note-ok-skill', batchUploadCodeEnvFiles);
|
||||
|
||||
const [result] = await invokeHandler(handler, [
|
||||
{
|
||||
id: 'call_prime_ok',
|
||||
name: Constants.SKILL_TOOL,
|
||||
args: { skillName: 'note-ok-skill' },
|
||||
},
|
||||
]);
|
||||
|
||||
expect(result.status).toBe('success');
|
||||
expect(result.content).not.toContain('could not be loaded');
|
||||
expect(result.artifact).toEqual(
|
||||
expect.objectContaining({
|
||||
session_id: 'session-ok',
|
||||
files: [
|
||||
expect.objectContaining({
|
||||
id: 'file-ok',
|
||||
name: 'skills/note-ok-skill/references/style.md',
|
||||
kind: 'skill',
|
||||
}),
|
||||
],
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("read_file pins lookup to the primed skill's _id when manually invoked this turn (no shadowing on collision)", async () => {
|
||||
/* Same-name collision corner: the resolver primed a specific doc
|
||||
(its `_id` is in `skillPrimedIdsByName`). If read_file used
|
||||
|
|
|
|||
|
|
@ -13,7 +13,7 @@ import type {
|
|||
} from '@librechat/agents';
|
||||
import type { StructuredToolInterface } from '@librechat/agents/langchain/tools';
|
||||
import type { CodeEnvRef } from 'librechat-data-provider';
|
||||
import type { SkillFileRecord } from './skillFiles';
|
||||
import type { SkillFileRecord, PrimeSkillFilesResult } from './skillFiles';
|
||||
import type { ServerRequest } from '~/types';
|
||||
import {
|
||||
backgroundTaskRegistry,
|
||||
|
|
@ -3419,7 +3419,7 @@ async function handleSkillToolCall(
|
|||
|
||||
const injectedMessages: InjectedMessage[] = [buildSkillPrimeMessage({ name: skill.name, body })];
|
||||
|
||||
const contentText = `Skill "${args.skillName}" loaded. Follow the instructions below.`;
|
||||
let contentText = `Skill "${args.skillName}" loaded. Follow the instructions below.`;
|
||||
let artifact:
|
||||
| {
|
||||
session_id: string;
|
||||
|
|
@ -3448,9 +3448,10 @@ async function handleSkillToolCall(
|
|||
getStrategyFunctions &&
|
||||
batchUploadCodeEnvFiles
|
||||
) {
|
||||
let primeResult: PrimeSkillFilesResult | null = null;
|
||||
try {
|
||||
const skillFiles = await listSkillFiles(skill._id);
|
||||
const primeResult = await primeSkillFiles({
|
||||
primeResult = await primeSkillFiles({
|
||||
skill,
|
||||
skillFiles,
|
||||
req,
|
||||
|
|
@ -3488,6 +3489,15 @@ async function handleSkillToolCall(
|
|||
error instanceof Error ? error.message : error,
|
||||
);
|
||||
}
|
||||
if (!primeResult) {
|
||||
/* Degrade loudly: without this note the model follows skill
|
||||
* instructions referencing sandbox paths that were never mounted
|
||||
* and burns turns on missing-path errors. */
|
||||
contentText +=
|
||||
`\n\nNote: this skill's bundled files could not be loaded into the code environment ` +
|
||||
`(upload failed or was rate-limited). Paths under /mnt/data/${SKILL_FILE_PREFIX}${skill.name}/ ` +
|
||||
`are NOT available to bash or code execution this turn. Use the read_file tool to view bundled files instead.`;
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
|
|
|
|||
|
|
@ -321,37 +321,35 @@ describe('primeInvokedSkills — execute_code capability gate', () => {
|
|||
* carry `resource_id` end-to-end, otherwise codeapi 400s with
|
||||
* `resource_id is invalid` (`type: 'undefined'`). Tests below lock
|
||||
* that contract on the lower-level helper directly. */
|
||||
function makeSkillFilesDeps(overrides: Partial<PrimeSkillFilesParams> = {}): PrimeSkillFilesParams {
|
||||
return {
|
||||
skill: {
|
||||
_id: SKILL_ID,
|
||||
name: 'brand-guidelines',
|
||||
body: 'skill body',
|
||||
version: SKILL_VERSION,
|
||||
},
|
||||
skillFiles: [],
|
||||
req: { user: { id: 'user-1' } } as PrimeSkillFilesParams['req'],
|
||||
getStrategyFunctions: jest.fn().mockReturnValue({
|
||||
getDownloadStream: jest.fn().mockResolvedValue(Readable.from(Buffer.from(''))),
|
||||
}),
|
||||
batchUploadCodeEnvFiles: jest.fn().mockResolvedValue({
|
||||
storage_session_id: 'session-fresh',
|
||||
files: [
|
||||
{ fileId: 'file-fresh', filename: 'skills/brand-guidelines/references/style.md' },
|
||||
{ fileId: 'file-skillmd', filename: 'skills/brand-guidelines/SKILL.md' },
|
||||
],
|
||||
}),
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
describe('primeSkillFiles — resource identity propagation', () => {
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
});
|
||||
|
||||
function makeSkillFilesDeps(
|
||||
overrides: Partial<PrimeSkillFilesParams> = {},
|
||||
): PrimeSkillFilesParams {
|
||||
return {
|
||||
skill: {
|
||||
_id: SKILL_ID,
|
||||
name: 'brand-guidelines',
|
||||
body: 'skill body',
|
||||
version: SKILL_VERSION,
|
||||
},
|
||||
skillFiles: [],
|
||||
req: { user: { id: 'user-1' } } as PrimeSkillFilesParams['req'],
|
||||
getStrategyFunctions: jest.fn().mockReturnValue({
|
||||
getDownloadStream: jest.fn().mockResolvedValue(Readable.from(Buffer.from(''))),
|
||||
}),
|
||||
batchUploadCodeEnvFiles: jest.fn().mockResolvedValue({
|
||||
storage_session_id: 'session-fresh',
|
||||
files: [
|
||||
{ fileId: 'file-fresh', filename: 'skills/brand-guidelines/references/style.md' },
|
||||
{ fileId: 'file-skillmd', filename: 'skills/brand-guidelines/SKILL.md' },
|
||||
],
|
||||
}),
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
it('fresh-upload path: emits resource_id=skill._id, kind=skill, version on each file', async () => {
|
||||
const deps = makeSkillFilesDeps({
|
||||
skillFiles: [
|
||||
|
|
@ -419,3 +417,174 @@ describe('primeSkillFiles — resource identity propagation', () => {
|
|||
]);
|
||||
});
|
||||
});
|
||||
|
||||
/* Codeapi's upload limiter defaults to 30 requests per user per 5 minutes,
|
||||
* and a workflow with many cold skills used to fan out one unbounded batch
|
||||
* upload per skill. The suite below locks the three mitigations: process-
|
||||
* wide upload slots, single-flight per (skill, version), and a single
|
||||
* Retry-After-honoring retry on 429. */
|
||||
describe('primeSkillFiles — upload rate-limit resilience', () => {
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
});
|
||||
|
||||
const flush = () => new Promise((resolve) => setImmediate(resolve));
|
||||
|
||||
function deferred<T>() {
|
||||
let resolve!: (value: T) => void;
|
||||
let reject!: (error: unknown) => void;
|
||||
const promise = new Promise<T>((res, rej) => {
|
||||
resolve = res;
|
||||
reject = rej;
|
||||
});
|
||||
return { promise, resolve, reject };
|
||||
}
|
||||
|
||||
function styleFileRecord() {
|
||||
return {
|
||||
relativePath: 'references/style.md',
|
||||
filename: 'style.md',
|
||||
filepath: '/storage/brand-guidelines/references/style.md',
|
||||
source: 's3',
|
||||
bytes: 256,
|
||||
};
|
||||
}
|
||||
|
||||
function uploadResult(skillName = 'brand-guidelines') {
|
||||
return {
|
||||
storage_session_id: `session-${skillName}`,
|
||||
files: [
|
||||
{ fileId: `file-${skillName}`, filename: `skills/${skillName}/references/style.md` },
|
||||
{ fileId: `skillmd-${skillName}`, filename: `skills/${skillName}/SKILL.md` },
|
||||
],
|
||||
};
|
||||
}
|
||||
|
||||
function rateLimit429(retryAfter: string) {
|
||||
const error = new Error('Request failed with status code 429') as Error & {
|
||||
isAxiosError: boolean;
|
||||
response: { status: number; headers: Record<string, string> };
|
||||
};
|
||||
error.isAxiosError = true;
|
||||
error.response = { status: 429, headers: { 'retry-after': retryAfter } };
|
||||
return error;
|
||||
}
|
||||
|
||||
it('single-flights concurrent primes of the same skill+version, clearing the flight on settle', async () => {
|
||||
const gate = deferred<ReturnType<typeof uploadResult>>();
|
||||
const batchUploadCodeEnvFiles = jest.fn().mockReturnValue(gate.promise);
|
||||
|
||||
const first = primeSkillFiles(
|
||||
makeSkillFilesDeps({ skillFiles: [styleFileRecord()], batchUploadCodeEnvFiles }),
|
||||
);
|
||||
const second = primeSkillFiles(
|
||||
makeSkillFilesDeps({ skillFiles: [styleFileRecord()], batchUploadCodeEnvFiles }),
|
||||
);
|
||||
await flush();
|
||||
expect(batchUploadCodeEnvFiles).toHaveBeenCalledTimes(1);
|
||||
|
||||
gate.resolve(uploadResult());
|
||||
const [firstResult, secondResult] = await Promise.all([first, second]);
|
||||
/* Joiners share the leader's result object, not a re-upload. */
|
||||
expect(firstResult).toBe(secondResult);
|
||||
expect(firstResult?.files).toHaveLength(1);
|
||||
|
||||
await primeSkillFiles(
|
||||
makeSkillFilesDeps({ skillFiles: [styleFileRecord()], batchUploadCodeEnvFiles }),
|
||||
);
|
||||
expect(batchUploadCodeEnvFiles).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('primes different versions of the same skill independently', async () => {
|
||||
const batchUploadCodeEnvFiles = jest.fn().mockResolvedValue(uploadResult());
|
||||
await Promise.all([
|
||||
primeSkillFiles(
|
||||
makeSkillFilesDeps({ skillFiles: [styleFileRecord()], batchUploadCodeEnvFiles }),
|
||||
),
|
||||
primeSkillFiles(
|
||||
makeSkillFilesDeps({
|
||||
skill: {
|
||||
_id: SKILL_ID,
|
||||
name: 'brand-guidelines',
|
||||
body: 'skill body',
|
||||
version: SKILL_VERSION + 1,
|
||||
},
|
||||
skillFiles: [styleFileRecord()],
|
||||
batchUploadCodeEnvFiles,
|
||||
}),
|
||||
),
|
||||
]);
|
||||
expect(batchUploadCodeEnvFiles).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('retries once on 429 within the Retry-After cap, re-acquiring streams per attempt', async () => {
|
||||
const getDownloadStream = jest.fn(async () => Readable.from(Buffer.from('')));
|
||||
const batchUploadCodeEnvFiles = jest
|
||||
.fn()
|
||||
.mockRejectedValueOnce(rateLimit429('0'))
|
||||
.mockResolvedValueOnce(uploadResult());
|
||||
|
||||
const result = await primeSkillFiles(
|
||||
makeSkillFilesDeps({
|
||||
skillFiles: [styleFileRecord()],
|
||||
batchUploadCodeEnvFiles,
|
||||
getStrategyFunctions: jest.fn().mockReturnValue({ getDownloadStream }),
|
||||
}),
|
||||
);
|
||||
|
||||
expect(batchUploadCodeEnvFiles).toHaveBeenCalledTimes(2);
|
||||
/* A consumed stream cannot be replayed — each attempt opens fresh ones. */
|
||||
expect(getDownloadStream).toHaveBeenCalledTimes(2);
|
||||
expect(result?.files).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('does not retry when Retry-After exceeds the cap', async () => {
|
||||
const batchUploadCodeEnvFiles = jest.fn().mockRejectedValue(rateLimit429('300'));
|
||||
const result = await primeSkillFiles(
|
||||
makeSkillFilesDeps({ skillFiles: [styleFileRecord()], batchUploadCodeEnvFiles }),
|
||||
);
|
||||
expect(batchUploadCodeEnvFiles).toHaveBeenCalledTimes(1);
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
|
||||
it('does not retry non-429 failures', async () => {
|
||||
const batchUploadCodeEnvFiles = jest.fn().mockRejectedValue(new Error('boom'));
|
||||
const result = await primeSkillFiles(
|
||||
makeSkillFilesDeps({ skillFiles: [styleFileRecord()], batchUploadCodeEnvFiles }),
|
||||
);
|
||||
expect(batchUploadCodeEnvFiles).toHaveBeenCalledTimes(1);
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
|
||||
it('bounds concurrent batch uploads to 3 process-wide slots', async () => {
|
||||
const gates = Array.from({ length: 5 }, () => deferred<ReturnType<typeof uploadResult>>());
|
||||
let uploadIndex = 0;
|
||||
const batchUploadCodeEnvFiles = jest
|
||||
.fn()
|
||||
.mockImplementation(() => gates[uploadIndex++].promise);
|
||||
const skillNames = Array.from({ length: 5 }, (_, i) => `skill-${i}`);
|
||||
|
||||
const primes = skillNames.map((name) =>
|
||||
primeSkillFiles(
|
||||
makeSkillFilesDeps({
|
||||
skill: { _id: new Types.ObjectId(), name, body: 'skill body', version: 1 },
|
||||
skillFiles: [styleFileRecord()],
|
||||
batchUploadCodeEnvFiles,
|
||||
}),
|
||||
),
|
||||
);
|
||||
|
||||
await flush();
|
||||
expect(batchUploadCodeEnvFiles).toHaveBeenCalledTimes(3);
|
||||
|
||||
gates[0].resolve(uploadResult(skillNames[0]));
|
||||
await flush();
|
||||
expect(batchUploadCodeEnvFiles).toHaveBeenCalledTimes(4);
|
||||
|
||||
for (let i = 1; i < gates.length; i++) {
|
||||
gates[i].resolve(uploadResult(skillNames[i]));
|
||||
}
|
||||
const results = await Promise.all(primes);
|
||||
expect(results.every((r) => r !== null)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,13 +1,14 @@
|
|||
import { Readable } from 'stream';
|
||||
import { isAxiosError } from 'axios';
|
||||
import { Constants } from '@librechat/agents';
|
||||
import { logger } from '@librechat/data-schemas';
|
||||
import type { ToolSessionMap, CodeSessionContext } from '@librechat/agents';
|
||||
import type { CodeEnvRef } from 'librechat-data-provider';
|
||||
import type { Types } from 'mongoose';
|
||||
import type { ServerRequest } from '~/types';
|
||||
import { createConcurrencyLimiter, logAxiosError } from '~/utils';
|
||||
import { extractInvokedSkillsFromPayload } from './run';
|
||||
import { SKILL_FILE_PREFIX } from './skills';
|
||||
import { logAxiosError } from '~/utils';
|
||||
|
||||
export interface SkillFileRecord {
|
||||
relativePath: string;
|
||||
|
|
@ -86,6 +87,86 @@ export interface PrimeSkillFilesResult {
|
|||
}>;
|
||||
}
|
||||
|
||||
/** Cap on concurrent skill batch uploads per process. Bounds burst pressure
|
||||
* on codeapi's per-user upload limiter (default 30 requests / 5 min). */
|
||||
const SKILL_UPLOAD_CONCURRENCY = 3;
|
||||
|
||||
/** Retry a 429'd upload only when the server's Retry-After fits under this
|
||||
* cap; a longer wait would stall a live chat turn worse than degrading. */
|
||||
const MAX_RETRY_AFTER_MS = 15_000;
|
||||
|
||||
const uploadSlots = createConcurrencyLimiter(SKILL_UPLOAD_CONCURRENCY);
|
||||
const inflightPrimes = new Map<string, Promise<PrimeSkillFilesResult | null>>();
|
||||
|
||||
type SkillUploadFiles = Array<{ stream: NodeJS.ReadableStream; filename: string }>;
|
||||
|
||||
function getRetryAfterMs(error: unknown): number | null {
|
||||
if (!isAxiosError(error) || error.response?.status !== 429) {
|
||||
return null;
|
||||
}
|
||||
const header = error.response.headers?.['retry-after'];
|
||||
const seconds = Number(Array.isArray(header) ? header[0] : header);
|
||||
if (!Number.isFinite(seconds) || seconds < 0) {
|
||||
return null;
|
||||
}
|
||||
return seconds * 1000;
|
||||
}
|
||||
|
||||
/** Single retry on 429, honoring Retry-After up to MAX_RETRY_AFTER_MS.
|
||||
* Runs inside an upload slot so the wait also brakes queued uploads. */
|
||||
async function retryOn429<T>(attempt: () => Promise<T>, label: string): Promise<T> {
|
||||
try {
|
||||
return await attempt();
|
||||
} catch (error) {
|
||||
const retryAfterMs = getRetryAfterMs(error);
|
||||
if (retryAfterMs == null || retryAfterMs > MAX_RETRY_AFTER_MS) {
|
||||
throw error;
|
||||
}
|
||||
logger.warn(`[primeSkillFiles] Rate-limited priming ${label}; retrying in ${retryAfterMs}ms`);
|
||||
await new Promise((resolve) => setTimeout(resolve, retryAfterMs));
|
||||
return attempt();
|
||||
}
|
||||
}
|
||||
|
||||
/** Opens SKILL.md and bundled-file streams for one upload attempt. Called
|
||||
* per attempt — a failed upload consumes the streams, so a retry must
|
||||
* re-acquire them. */
|
||||
async function collectSkillUploadFiles(params: PrimeSkillFilesParams): Promise<SkillUploadFiles> {
|
||||
const { skill, skillFiles, req, getStrategyFunctions } = params;
|
||||
const filesToUpload: SkillUploadFiles = [];
|
||||
|
||||
// SKILL.md from the skill body
|
||||
const bodyBuffer = Buffer.from(skill.body, 'utf-8');
|
||||
filesToUpload.push({
|
||||
stream: Readable.from(bodyBuffer),
|
||||
filename: `${SKILL_FILE_PREFIX}${skill.name}/SKILL.md`,
|
||||
});
|
||||
|
||||
// Bundled files from storage (parallel stream acquisition)
|
||||
const streamResults = await Promise.allSettled(
|
||||
skillFiles.map(async (file) => {
|
||||
const strategy = getStrategyFunctions(file.source);
|
||||
if (!strategy.getDownloadStream) {
|
||||
logger.warn(
|
||||
`[primeSkillFiles] No download stream for "${file.relativePath}" (source: ${file.source})`,
|
||||
);
|
||||
return null;
|
||||
}
|
||||
const stream = await strategy.getDownloadStream(req, file.filepath);
|
||||
return { stream, filename: `${SKILL_FILE_PREFIX}${skill.name}/${file.relativePath}` };
|
||||
}),
|
||||
);
|
||||
for (const result of streamResults) {
|
||||
if (result.status === 'fulfilled' && result.value) {
|
||||
filesToUpload.push(result.value);
|
||||
} else if (result.status === 'rejected') {
|
||||
logger.error('[primeSkillFiles] Failed to get stream:', result.reason);
|
||||
}
|
||||
}
|
||||
|
||||
return filesToUpload;
|
||||
}
|
||||
|
||||
/**
|
||||
* Uploads skill files to the code execution environment.
|
||||
*
|
||||
|
|
@ -95,15 +176,39 @@ export interface PrimeSkillFilesResult {
|
|||
*
|
||||
* After upload, persists new codeEnvIdentifiers on the SkillFile
|
||||
* documents for future freshness checks.
|
||||
*
|
||||
* Rate-limit resilience: concurrent primes of the same (skill, version)
|
||||
* share one flight, uploads are bounded process-wide, and a 429 retries
|
||||
* once per the server's Retry-After.
|
||||
*/
|
||||
export async function primeSkillFiles(
|
||||
params: PrimeSkillFilesParams,
|
||||
): Promise<PrimeSkillFilesResult | null> {
|
||||
/* Single-flight per (skill, version): concurrent primes of the same cold
|
||||
* skill join the in-flight upload instead of double-spending the upload
|
||||
* rate budget. Skill _ids are tenant-scoped and the resulting session is
|
||||
* resource-scoped (`<tenant>:skill:<id>:v:<version>`), so sharing the
|
||||
* result across requests is sound. Per-process best-effort; the awaited
|
||||
* codeEnvRef persist covers cross-turn and cross-node dedupe. */
|
||||
const flightKey = `${params.skill._id}:v:${params.skill.version}`;
|
||||
const inflight = inflightPrimes.get(flightKey);
|
||||
if (inflight) {
|
||||
return inflight;
|
||||
}
|
||||
const flight = executePrimeSkillFiles(params).finally(() => {
|
||||
inflightPrimes.delete(flightKey);
|
||||
});
|
||||
inflightPrimes.set(flightKey, flight);
|
||||
return flight;
|
||||
}
|
||||
|
||||
async function executePrimeSkillFiles(
|
||||
params: PrimeSkillFilesParams,
|
||||
): Promise<PrimeSkillFilesResult | null> {
|
||||
const {
|
||||
skill,
|
||||
skillFiles,
|
||||
req,
|
||||
getStrategyFunctions,
|
||||
batchUploadCodeEnvFiles,
|
||||
getSessionInfo,
|
||||
checkIfActive,
|
||||
|
|
@ -171,63 +276,43 @@ export async function primeSkillFiles(
|
|||
}
|
||||
}
|
||||
|
||||
// Collect streams for batch upload
|
||||
const filesToUpload: Array<{ stream: NodeJS.ReadableStream; filename: string }> = [];
|
||||
|
||||
// SKILL.md from the skill body
|
||||
const bodyBuffer = Buffer.from(skill.body, 'utf-8');
|
||||
filesToUpload.push({
|
||||
stream: Readable.from(bodyBuffer),
|
||||
filename: `${SKILL_FILE_PREFIX}${skill.name}/SKILL.md`,
|
||||
});
|
||||
|
||||
// Bundled files from storage (parallel stream acquisition)
|
||||
const streamResults = await Promise.allSettled(
|
||||
skillFiles.map(async (file) => {
|
||||
const strategy = getStrategyFunctions(file.source);
|
||||
if (!strategy.getDownloadStream) {
|
||||
logger.warn(
|
||||
`[primeSkillFiles] No download stream for "${file.relativePath}" (source: ${file.source})`,
|
||||
);
|
||||
return null;
|
||||
}
|
||||
const stream = await strategy.getDownloadStream(req, file.filepath);
|
||||
return { stream, filename: `${SKILL_FILE_PREFIX}${skill.name}/${file.relativePath}` };
|
||||
}),
|
||||
);
|
||||
for (const result of streamResults) {
|
||||
if (result.status === 'fulfilled' && result.value) {
|
||||
filesToUpload.push(result.value);
|
||||
} else if (result.status === 'rejected') {
|
||||
logger.error('[primeSkillFiles] Failed to get stream:', result.reason);
|
||||
}
|
||||
}
|
||||
|
||||
if (filesToUpload.length === 0) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const entityId = skill._id.toString();
|
||||
try {
|
||||
const entityId = skill._id.toString();
|
||||
const result = await batchUploadCodeEnvFiles({
|
||||
req,
|
||||
files: filesToUpload,
|
||||
/* Resource identity for codeapi's sessionKey: skill files share
|
||||
* cross-user-within-tenant under `<tenant>:skill:<id>:v:<version>`.
|
||||
* Bumping `skill.version` on edit naturally invalidates the prior
|
||||
* cache entry under the new sessionKey. */
|
||||
kind: 'skill',
|
||||
id: entityId,
|
||||
version: skill.version,
|
||||
/* Skill files are infrastructure: SKILL.md + bundled scripts/schemas/
|
||||
* docs that the agent reads but should never edit. Tag the upload as
|
||||
* read-only so codeapi seals the inputs (chmod 444 in-sandbox) and
|
||||
* walker echoes the original refs as `inherited: true` even if some
|
||||
* sandboxed code path mutates bytes on disk. Without this, modified
|
||||
* skill files surface as ghost generated artifacts the user has no
|
||||
* authority to download. */
|
||||
read_only: true,
|
||||
});
|
||||
/* Streams open inside the slot (not while queued) and inside the retry
|
||||
* closure (a failed attempt consumes them). The slot bounds concurrent
|
||||
* uploads process-wide across both prime call sites. */
|
||||
const uploaded = await uploadSlots(() =>
|
||||
retryOn429(async () => {
|
||||
const filesToUpload = await collectSkillUploadFiles(params);
|
||||
if (filesToUpload.length === 0) {
|
||||
return null;
|
||||
}
|
||||
const result = await batchUploadCodeEnvFiles({
|
||||
req,
|
||||
files: filesToUpload,
|
||||
/* Resource identity for codeapi's sessionKey: skill files share
|
||||
* cross-user-within-tenant under `<tenant>:skill:<id>:v:<version>`.
|
||||
* Bumping `skill.version` on edit naturally invalidates the prior
|
||||
* cache entry under the new sessionKey. */
|
||||
kind: 'skill',
|
||||
id: entityId,
|
||||
version: skill.version,
|
||||
/* Skill files are infrastructure: SKILL.md + bundled scripts/schemas/
|
||||
* docs that the agent reads but should never edit. Tag the upload as
|
||||
* read-only so codeapi seals the inputs (chmod 444 in-sandbox) and
|
||||
* walker echoes the original refs as `inherited: true` even if some
|
||||
* sandboxed code path mutates bytes on disk. Without this, modified
|
||||
* skill files surface as ghost generated artifacts the user has no
|
||||
* authority to download. */
|
||||
read_only: true,
|
||||
});
|
||||
return { filesToUpload, result };
|
||||
}, `skill "${skill.name}"`),
|
||||
);
|
||||
if (uploaded == null) {
|
||||
return null;
|
||||
}
|
||||
const { filesToUpload, result } = uploaded;
|
||||
// Exclude SKILL.md from the returned files array — it is uploaded to disk
|
||||
// for bash access but has no codeEnvRef (cannot be cached). Omitting it
|
||||
// here keeps the fresh-upload and cache-hit code paths consistent.
|
||||
|
|
@ -525,6 +610,12 @@ export async function primeInvokedSkills(
|
|||
}
|
||||
} else if (r.status === 'rejected') {
|
||||
logger.warn('[primeInvokedSkills] Failed to prime skill files:', r.reason);
|
||||
} else {
|
||||
/* Fulfilled-null: primeSkillFiles swallowed an upload failure (429,
|
||||
* partial batch). The run proceeds without this skill's files. */
|
||||
logger.warn(
|
||||
`[primeInvokedSkills] Priming returned no files for skill "${r.value.skill.name}"`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue