mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-10-25 03:32:12 +00:00
🪛 feat: read_file Sandbox Fallback for /mnt/data + Non-Skill Paths
When the model called `read_file` with a code-execution path (e.g.
`/mnt/data/sentinel.txt`), the handler returned a misleading
`Use format: {skillName}/{path}` error. Adds a sandbox-aware fallback:
- Short-circuit `/mnt/data/...` (can never be a skill reference) →
route to a sandbox `cat` via the new host-provided `readSandboxFile`
callback, which POSTs to the codeapi `/exec` endpoint.
- Skip the skill resolver entirely when `accessibleSkillIds` is empty
— the resolved-output of `resolveAgentScopedSkillIds` already
collapses the admin capability + ephemeral badge + persisted
`skills_enabled` chain, so an empty value is the authoritative
"skills aren't in scope for this agent" signal.
- For `{firstSegment}/...` paths, consult the catalog-derived
`activeSkillNames` Set (no DB read) to detect non-skill names and
fall through to the sandbox before the model has to retry with
`bash_tool`.
`activeSkillNames` is captured from `injectSkillCatalog`, threaded onto
`InitializedAgent`, into `agentToolContexts`, then through
`enrichWithSkillConfigurable` into `mergedConfigurable` for the handler.
The host implementation of `readSandboxFile` lives in
`api/server/services/Files/Code/process.js` and shells `cat <path>`
through the seeded sandbox session — `tc.codeSessionContext`
(emitted by ToolNode for `read_file` calls in `@librechat/agents`
v3.1.72+) provides the `session_id` + `_injected_files` so the read
lands in the same sandbox that holds prior-turn artifacts. When the
seeded context isn't available (older agents version, no codeapi
configured), the handler returns a model-visible error pointing at
`bash_tool` instead of silently failing.
Tests: 8 new `handleReadFileCall` cases cover the new short-circuits,
the skills-not-enabled gate, the activeSkillNames lookup, the
sandbox-fallback success path, and the bash_tool retry hint on
fallback failure. Existing `read_file` tests now opt into "skills are
in scope" via a `skillsInScope()` fixture (production wouldn't reach
the skill lookup with empty `accessibleSkillIds`).
This commit is contained in:
parent
1e055949a9
commit
fdc207aa1e
8 changed files with 539 additions and 7 deletions
|
|
@ -186,6 +186,7 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => {
|
|||
ctx.accessibleSkillIds,
|
||||
ctx.codeEnvAvailable === true,
|
||||
ctx.skillPrimedIdsByName,
|
||||
ctx.activeSkillNames,
|
||||
);
|
||||
},
|
||||
toolEndCallback,
|
||||
|
|
@ -324,6 +325,7 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => {
|
|||
tool_resources: primaryConfig.tool_resources,
|
||||
actionsEnabled: primaryConfig.actionsEnabled,
|
||||
accessibleSkillIds: primaryConfig.accessibleSkillIds,
|
||||
activeSkillNames: primaryConfig.activeSkillNames,
|
||||
codeEnvAvailable: primaryConfig.codeEnvAvailable,
|
||||
skillPrimedIdsByName,
|
||||
});
|
||||
|
|
@ -397,6 +399,7 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => {
|
|||
tool_resources: config.tool_resources,
|
||||
actionsEnabled: config.actionsEnabled,
|
||||
accessibleSkillIds: config.accessibleSkillIds,
|
||||
activeSkillNames: config.activeSkillNames,
|
||||
codeEnvAvailable: config.codeEnvAvailable,
|
||||
skillPrimedIdsByName: buildSkillPrimedIdsByName(
|
||||
config.manualSkillPrimes,
|
||||
|
|
@ -456,6 +459,7 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => {
|
|||
tool_resources: config.tool_resources,
|
||||
actionsEnabled: config.actionsEnabled,
|
||||
accessibleSkillIds: config.accessibleSkillIds,
|
||||
activeSkillNames: config.activeSkillNames,
|
||||
codeEnvAvailable: config.codeEnvAvailable,
|
||||
});
|
||||
}
|
||||
|
|
@ -594,6 +598,7 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => {
|
|||
tool_resources: config.tool_resources,
|
||||
actionsEnabled: config.actionsEnabled,
|
||||
accessibleSkillIds: config.accessibleSkillIds,
|
||||
activeSkillNames: config.activeSkillNames,
|
||||
skillPrimedIdsByName: buildSkillPrimedIdsByName(
|
||||
config.manualSkillPrimes,
|
||||
config.alwaysApplySkillPrimes,
|
||||
|
|
|
|||
|
|
@ -1,6 +1,10 @@
|
|||
const { getStrategyFunctions } = require('~/server/services/Files/strategies');
|
||||
const { batchUploadCodeEnvFiles } = require('~/server/services/Files/Code/crud');
|
||||
const { getSessionInfo, checkIfActive } = require('~/server/services/Files/Code/process');
|
||||
const {
|
||||
getSessionInfo,
|
||||
checkIfActive,
|
||||
readSandboxFile,
|
||||
} = require('~/server/services/Files/Code/process');
|
||||
const { enrichWithSkillConfigurable } = require('@librechat/api');
|
||||
const db = require('~/models');
|
||||
|
||||
|
|
@ -66,6 +70,15 @@ const skillToolDeps = {
|
|||
updateSkillFileCodeEnvIds: db.updateSkillFileCodeEnvIds,
|
||||
getSkillFileByPath: db.getSkillFileByPath,
|
||||
updateSkillFileContent: db.updateSkillFileContent,
|
||||
/**
|
||||
* `read_file` falls back to a sandbox `cat` for `/mnt/data/...` paths
|
||||
* and for `{firstSegment}/...` paths whose first segment isn't a known
|
||||
* skill name. The handler routes through this when the agent has code
|
||||
* execution enabled; the codeapi base URL comes from
|
||||
* `LIBRECHAT_CODE_BASEURL` and the sandbox session id is forwarded by
|
||||
* the agents-side `ToolNode` via `tc.codeSessionContext`.
|
||||
*/
|
||||
readSandboxFile,
|
||||
};
|
||||
|
||||
function getSkillToolDeps() {
|
||||
|
|
|
|||
|
|
@ -466,9 +466,81 @@ const primeFiles = async (options) => {
|
|||
return { files, toolContext };
|
||||
};
|
||||
|
||||
/**
|
||||
* Reads a single file from the code-execution sandbox by shelling `cat`
|
||||
* through the sandbox `/exec` endpoint. Used by the `read_file` host
|
||||
* handler when the requested path is a code-env path (`/mnt/data/...`)
|
||||
* or otherwise not resolvable as a skill file. Resolves to
|
||||
* `{ content }` from stdout on success, or `null` when the codeapi base
|
||||
* URL isn't configured / the read returns no content (caller turns that
|
||||
* into a model-visible error). Throws axios-style errors on transport
|
||||
* failure so the caller can surface a meaningful error message.
|
||||
*
|
||||
* `session_id` and `files` come from the seeded `tc.codeSessionContext`
|
||||
* (emitted by the agents-side `ToolNode` for `read_file` calls in
|
||||
* v3.1.72+) so the read lands in the same sandbox session that holds
|
||||
* the agent's prior-turn artifacts.
|
||||
*
|
||||
* @param {Object} params
|
||||
* @param {string} params.file_path - Absolute path inside the sandbox (e.g. `/mnt/data/foo.txt`).
|
||||
* @param {string} [params.session_id] - Sandbox session id from the seeded context.
|
||||
* @param {Array<{id: string, name: string, session_id?: string}>} [params.files] - File refs to mount.
|
||||
* @returns {Promise<{content: string} | null>}
|
||||
*/
|
||||
async function readSandboxFile({ file_path, session_id, files }) {
|
||||
const baseURL = getCodeBaseURL();
|
||||
if (!baseURL) {
|
||||
return null;
|
||||
}
|
||||
|
||||
/** Single-quote `file_path` with embedded-quote escaping so a malicious
|
||||
* filename can't break out of the `cat` command. The handler upstream
|
||||
* has already established this is a code-env path the model
|
||||
* legitimately asked to read; this just keeps the shell quoting safe. */
|
||||
const safePath = `'${file_path.replace(/'/g, `'\\''`)}'`;
|
||||
/** @type {Record<string, unknown>} */
|
||||
const postData = { lang: 'bash', code: `cat ${safePath}` };
|
||||
if (session_id) {
|
||||
postData.session_id = session_id;
|
||||
}
|
||||
if (files && files.length > 0) {
|
||||
postData.files = files;
|
||||
}
|
||||
|
||||
try {
|
||||
const response = await axios({
|
||||
method: 'post',
|
||||
url: `${baseURL}/exec`,
|
||||
data: postData,
|
||||
headers: {
|
||||
'Content-Type': 'application/json',
|
||||
'User-Agent': 'LibreChat/1.0',
|
||||
},
|
||||
httpAgent: codeServerHttpAgent,
|
||||
httpsAgent: codeServerHttpsAgent,
|
||||
timeout: 15000,
|
||||
});
|
||||
const result = response?.data ?? {};
|
||||
if (result.stderr && (result.stdout == null || result.stdout === '')) {
|
||||
throw new Error(String(result.stderr).trim());
|
||||
}
|
||||
if (result.stdout == null) {
|
||||
return null;
|
||||
}
|
||||
return { content: String(result.stdout) };
|
||||
} catch (error) {
|
||||
logAxiosError({
|
||||
message: `Error reading sandbox file "${file_path}"`,
|
||||
error,
|
||||
});
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
module.exports = {
|
||||
primeFiles,
|
||||
checkIfActive,
|
||||
getSessionInfo,
|
||||
processCodeOutput,
|
||||
readSandboxFile,
|
||||
};
|
||||
|
|
|
|||
|
|
@ -177,9 +177,25 @@ describe('createToolExecuteHandler', () => {
|
|||
});
|
||||
|
||||
describe('skill tool model-invocation gate', () => {
|
||||
/**
|
||||
* Sentinel non-empty `accessibleSkillIds` for fixtures that exercise the
|
||||
* existing skill-resolution path. The `read_file` handler short-circuits
|
||||
* to the sandbox fallback (or errors when code env isn't available)
|
||||
* whenever `accessibleSkillIds` is empty — the resolved-output of the
|
||||
* `resolveAgentScopedSkillIds` chain (admin capability + ephemeral badge
|
||||
* / persisted `skills_enabled` + ACL). Tests that mock `getSkillByName`
|
||||
* directly need to opt in to "skills are in scope" so they reach the
|
||||
* lookup; otherwise they'd hit the gate with an unconfigured runtime.
|
||||
*/
|
||||
const skillsInScope = (): unknown[] => {
|
||||
const { Types } = jest.requireActual('mongoose') as typeof import('mongoose');
|
||||
return [new Types.ObjectId()];
|
||||
};
|
||||
|
||||
function createSkillHandler(getSkillByName: ToolExecuteOptions['getSkillByName']) {
|
||||
const loadTools: ToolExecuteOptions['loadTools'] = jest.fn(async () => ({
|
||||
loadedTools: [],
|
||||
configurable: { accessibleSkillIds: skillsInScope() },
|
||||
}));
|
||||
return createToolExecuteHandler({ loadTools, getSkillByName });
|
||||
}
|
||||
|
|
@ -289,7 +305,10 @@ describe('createToolExecuteHandler', () => {
|
|||
fileCount: 0,
|
||||
}));
|
||||
const handler = createToolExecuteHandler({
|
||||
loadTools: jest.fn(async () => ({ loadedTools: [] })),
|
||||
loadTools: jest.fn(async () => ({
|
||||
loadedTools: [],
|
||||
configurable: { accessibleSkillIds: skillsInScope() },
|
||||
})),
|
||||
getSkillByName,
|
||||
});
|
||||
|
||||
|
|
@ -329,6 +348,7 @@ describe('createToolExecuteHandler', () => {
|
|||
loadTools: jest.fn(async () => ({
|
||||
loadedTools: [],
|
||||
configurable: {
|
||||
accessibleSkillIds: skillsInScope(),
|
||||
skillPrimedIdsByName: { 'manually-primed': primedHex },
|
||||
},
|
||||
})),
|
||||
|
|
@ -375,7 +395,10 @@ describe('createToolExecuteHandler', () => {
|
|||
disableModelInvocation: true,
|
||||
}));
|
||||
const handler = createToolExecuteHandler({
|
||||
loadTools: jest.fn(async () => ({ loadedTools: [] })),
|
||||
loadTools: jest.fn(async () => ({
|
||||
loadedTools: [],
|
||||
configurable: { accessibleSkillIds: skillsInScope() },
|
||||
})),
|
||||
getSkillByName,
|
||||
getSkillFileByPath: jest.fn(),
|
||||
});
|
||||
|
|
@ -401,7 +424,10 @@ describe('createToolExecuteHandler', () => {
|
|||
fileCount: 0,
|
||||
}));
|
||||
const handler = createToolExecuteHandler({
|
||||
loadTools: jest.fn(async () => ({ loadedTools: [] })),
|
||||
loadTools: jest.fn(async () => ({
|
||||
loadedTools: [],
|
||||
configurable: { accessibleSkillIds: skillsInScope() },
|
||||
})),
|
||||
getSkillByName,
|
||||
});
|
||||
|
||||
|
|
@ -435,6 +461,7 @@ describe('createToolExecuteHandler', () => {
|
|||
loadTools: jest.fn(async () => ({
|
||||
loadedTools: [],
|
||||
configurable: {
|
||||
accessibleSkillIds: skillsInScope(),
|
||||
skillPrimedIdsByName: { 'manual-only-skill': '507f1f77bcf86cd799439020' },
|
||||
},
|
||||
})),
|
||||
|
|
@ -469,6 +496,7 @@ describe('createToolExecuteHandler', () => {
|
|||
loadTools: jest.fn(async () => ({
|
||||
loadedTools: [],
|
||||
configurable: {
|
||||
accessibleSkillIds: skillsInScope(),
|
||||
skillPrimedIdsByName: { 'something-else': '507f1f77bcf86cd799439030' },
|
||||
},
|
||||
})),
|
||||
|
|
@ -505,6 +533,7 @@ describe('createToolExecuteHandler', () => {
|
|||
loadTools: jest.fn(async () => ({
|
||||
loadedTools: [],
|
||||
configurable: {
|
||||
accessibleSkillIds: skillsInScope(),
|
||||
/* Map includes the always-apply skill because `buildSkillPrimedIdsByName`
|
||||
now combines both prime sources. */
|
||||
skillPrimedIdsByName: {
|
||||
|
|
@ -543,6 +572,7 @@ describe('createToolExecuteHandler', () => {
|
|||
loadTools: jest.fn(async () => ({
|
||||
loadedTools: [],
|
||||
configurable: {
|
||||
accessibleSkillIds: skillsInScope(),
|
||||
skillPrimedIdsByName: { collides: primedHex },
|
||||
},
|
||||
})),
|
||||
|
|
@ -639,4 +669,232 @@ describe('createToolExecuteHandler', () => {
|
|||
expect(listSkillFiles).toHaveBeenCalledWith(SKILL_ID);
|
||||
});
|
||||
});
|
||||
|
||||
describe('read_file sandbox fallback (code-env paths + non-skill segments)', () => {
|
||||
/**
|
||||
* `accessibleSkillIds.length > 0` is the runtime signal "skills are
|
||||
* effectively in scope" — the resolver chain (admin capability +
|
||||
* ephemeral badge / persisted `skills_enabled` + ACL) collapses into
|
||||
* this single value at agent init.
|
||||
*/
|
||||
const skillsInScope = (): unknown[] => {
|
||||
const { Types } = jest.requireActual('mongoose') as typeof import('mongoose');
|
||||
return [new Types.ObjectId()];
|
||||
};
|
||||
|
||||
function makeReadFileHandler(params: {
|
||||
codeEnvAvailable?: boolean;
|
||||
accessibleSkillIds?: unknown[];
|
||||
activeSkillNames?: Set<string>;
|
||||
readSandboxFile?: ToolExecuteOptions['readSandboxFile'];
|
||||
getSkillByName?: ToolExecuteOptions['getSkillByName'];
|
||||
}) {
|
||||
const loadTools: ToolExecuteOptions['loadTools'] = jest.fn(async () => ({
|
||||
loadedTools: [],
|
||||
configurable: {
|
||||
codeEnvAvailable: params.codeEnvAvailable === true,
|
||||
accessibleSkillIds: params.accessibleSkillIds ?? [],
|
||||
activeSkillNames: params.activeSkillNames,
|
||||
},
|
||||
}));
|
||||
return createToolExecuteHandler({
|
||||
loadTools,
|
||||
getSkillByName: params.getSkillByName,
|
||||
readSandboxFile: params.readSandboxFile,
|
||||
});
|
||||
}
|
||||
|
||||
it('routes /mnt/data/ paths to the sandbox fallback when codeEnv is available', async () => {
|
||||
const readSandboxFile = jest.fn(async () => ({ content: 'hello-world' }));
|
||||
const handler = makeReadFileHandler({
|
||||
codeEnvAvailable: true,
|
||||
accessibleSkillIds: skillsInScope(),
|
||||
readSandboxFile,
|
||||
});
|
||||
|
||||
const [result] = await invokeHandler(handler, [
|
||||
{
|
||||
id: 'call_mnt_1',
|
||||
name: Constants.READ_FILE,
|
||||
args: { file_path: '/mnt/data/sentinel.txt' },
|
||||
codeSessionContext: {
|
||||
session_id: 'sess-X',
|
||||
files: [{ id: 'f1', name: 'sentinel.txt', session_id: 'sess-X' }],
|
||||
},
|
||||
} as unknown as ToolCallRequest,
|
||||
]);
|
||||
|
||||
expect(readSandboxFile).toHaveBeenCalledWith({
|
||||
file_path: '/mnt/data/sentinel.txt',
|
||||
session_id: 'sess-X',
|
||||
files: [{ id: 'f1', name: 'sentinel.txt', session_id: 'sess-X' }],
|
||||
});
|
||||
expect(result.status).toBe('success');
|
||||
expect(result.content).toContain('hello-world');
|
||||
});
|
||||
|
||||
it('returns a clear error for /mnt/data/ when codeEnv is not available', async () => {
|
||||
const readSandboxFile = jest.fn();
|
||||
const handler = makeReadFileHandler({
|
||||
codeEnvAvailable: false,
|
||||
accessibleSkillIds: skillsInScope(),
|
||||
readSandboxFile,
|
||||
});
|
||||
|
||||
const [result] = await invokeHandler(handler, [
|
||||
{
|
||||
id: 'call_mnt_2',
|
||||
name: Constants.READ_FILE,
|
||||
args: { file_path: '/mnt/data/sentinel.txt' },
|
||||
},
|
||||
]);
|
||||
|
||||
expect(readSandboxFile).not.toHaveBeenCalled();
|
||||
expect(result.status).toBe('error');
|
||||
expect(result.errorMessage).toContain('code-execution sandbox path');
|
||||
});
|
||||
|
||||
it('falls back to sandbox when first segment is not a known skill name', async () => {
|
||||
const readSandboxFile = jest.fn(async () => ({ content: 'sandbox-data' }));
|
||||
const handler = makeReadFileHandler({
|
||||
codeEnvAvailable: true,
|
||||
accessibleSkillIds: skillsInScope(),
|
||||
activeSkillNames: new Set(['only-real-skill']),
|
||||
readSandboxFile,
|
||||
});
|
||||
|
||||
const [result] = await invokeHandler(handler, [
|
||||
{
|
||||
id: 'call_unknown_skill',
|
||||
name: Constants.READ_FILE,
|
||||
args: { file_path: 'not-a-skill/foo.md' },
|
||||
codeSessionContext: { session_id: 'sess-Y' },
|
||||
} as unknown as ToolCallRequest,
|
||||
]);
|
||||
|
||||
expect(readSandboxFile).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ file_path: 'not-a-skill/foo.md', session_id: 'sess-Y' }),
|
||||
);
|
||||
expect(result.status).toBe('success');
|
||||
expect(result.content).toContain('sandbox-data');
|
||||
});
|
||||
|
||||
it('routes through sandbox when skills are not effectively enabled (empty accessibleSkillIds)', async () => {
|
||||
/**
|
||||
* `accessibleSkillIds: []` is what `resolveAgentScopedSkillIds`
|
||||
* returns when the admin capability is off, the ephemeral badge is
|
||||
* off, or the persisted agent has `skills_enabled !== true`. Skip
|
||||
* the skill resolver entirely.
|
||||
*/
|
||||
const readSandboxFile = jest.fn(async () => ({ content: 'sandbox-content' }));
|
||||
const getSkillByName = jest.fn();
|
||||
const handler = makeReadFileHandler({
|
||||
codeEnvAvailable: true,
|
||||
accessibleSkillIds: [],
|
||||
readSandboxFile,
|
||||
getSkillByName,
|
||||
});
|
||||
|
||||
const [result] = await invokeHandler(handler, [
|
||||
{
|
||||
id: 'call_skills_off',
|
||||
name: Constants.READ_FILE,
|
||||
args: { file_path: 'whatever/path.md' },
|
||||
},
|
||||
]);
|
||||
|
||||
expect(getSkillByName).not.toHaveBeenCalled();
|
||||
expect(readSandboxFile).toHaveBeenCalled();
|
||||
expect(result.status).toBe('success');
|
||||
expect(result.content).toContain('sandbox-content');
|
||||
});
|
||||
|
||||
it('returns a clear error when skills off + codeEnv off (nowhere to route)', async () => {
|
||||
const handler = makeReadFileHandler({
|
||||
codeEnvAvailable: false,
|
||||
accessibleSkillIds: [],
|
||||
});
|
||||
|
||||
const [result] = await invokeHandler(handler, [
|
||||
{
|
||||
id: 'call_no_route',
|
||||
name: Constants.READ_FILE,
|
||||
args: { file_path: 'whatever/path.md' },
|
||||
},
|
||||
]);
|
||||
|
||||
expect(result.status).toBe('error');
|
||||
expect(result.errorMessage).toContain('Skill files are not available');
|
||||
});
|
||||
|
||||
it('still resolves a real skill when activeSkillNames knows the name', async () => {
|
||||
const getSkillByName = jest.fn(async () => ({
|
||||
_id: 'real-skill-id' as unknown as never,
|
||||
name: 'real-skill',
|
||||
body: '# Real Body',
|
||||
fileCount: 0,
|
||||
}));
|
||||
const readSandboxFile = jest.fn();
|
||||
const handler = makeReadFileHandler({
|
||||
codeEnvAvailable: true,
|
||||
accessibleSkillIds: skillsInScope(),
|
||||
activeSkillNames: new Set(['real-skill']),
|
||||
readSandboxFile,
|
||||
getSkillByName,
|
||||
});
|
||||
|
||||
const [result] = await invokeHandler(handler, [
|
||||
{
|
||||
id: 'call_real_skill',
|
||||
name: Constants.READ_FILE,
|
||||
args: { file_path: 'real-skill/SKILL.md' },
|
||||
},
|
||||
]);
|
||||
|
||||
expect(getSkillByName).toHaveBeenCalled();
|
||||
expect(readSandboxFile).not.toHaveBeenCalled();
|
||||
expect(result.status).toBe('success');
|
||||
expect(result.content).toContain('Real Body');
|
||||
});
|
||||
|
||||
it('hints toward bash_tool when readSandboxFile is not configured', async () => {
|
||||
const handler = makeReadFileHandler({
|
||||
codeEnvAvailable: true,
|
||||
accessibleSkillIds: skillsInScope(),
|
||||
// readSandboxFile intentionally omitted
|
||||
});
|
||||
|
||||
const [result] = await invokeHandler(handler, [
|
||||
{
|
||||
id: 'call_no_callback',
|
||||
name: Constants.READ_FILE,
|
||||
args: { file_path: '/mnt/data/x.txt' },
|
||||
},
|
||||
]);
|
||||
|
||||
expect(result.status).toBe('error');
|
||||
expect(result.errorMessage).toContain('bash_tool');
|
||||
});
|
||||
|
||||
it('surfaces sandbox fallback failures with a bash_tool retry hint', async () => {
|
||||
const readSandboxFile = jest.fn(async () => null);
|
||||
const handler = makeReadFileHandler({
|
||||
codeEnvAvailable: true,
|
||||
accessibleSkillIds: skillsInScope(),
|
||||
readSandboxFile,
|
||||
});
|
||||
|
||||
const [result] = await invokeHandler(handler, [
|
||||
{
|
||||
id: 'call_null_result',
|
||||
name: Constants.READ_FILE,
|
||||
args: { file_path: '/mnt/data/missing.txt' },
|
||||
},
|
||||
]);
|
||||
|
||||
expect(result.status).toBe('error');
|
||||
expect(result.errorMessage).toContain('Failed to read');
|
||||
expect(result.errorMessage).toContain('bash_tool');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -119,6 +119,20 @@ export interface ToolExecuteOptions {
|
|||
relativePath: string,
|
||||
update: { content?: string; isBinary?: boolean },
|
||||
) => Promise<void>;
|
||||
/**
|
||||
* Reads a code-execution sandbox file by shelling `cat` through the
|
||||
* sandbox `/exec` endpoint. The host implementation supplies the
|
||||
* codeapi base URL + auth and forwards the seeded `session_id` and
|
||||
* `files` so the read lands in the same sandbox session that holds
|
||||
* the agent's prior-turn artifacts. Returns `null` when codeapi is
|
||||
* unavailable; throws on transport errors so the handler can surface
|
||||
* a meaningful error message to the model.
|
||||
*/
|
||||
readSandboxFile?: (params: {
|
||||
file_path: string;
|
||||
session_id?: string;
|
||||
files?: Array<{ id: string; name: string; session_id?: string }>;
|
||||
}) => Promise<{ content: string } | null>;
|
||||
}
|
||||
|
||||
const MAX_READABLE_BYTES = 262_144;
|
||||
|
|
@ -133,6 +147,65 @@ function addLineNumbers(content: string): string {
|
|||
return lines.map((l, i) => `${String(i + 1).padStart(w, ' ')} | ${l}`).join('\n');
|
||||
}
|
||||
|
||||
/**
|
||||
* Routes a `read_file` call to the code-execution sandbox via the
|
||||
* host-provided `readSandboxFile` callback. The sandbox session id and
|
||||
* primed file refs come from `tc.codeSessionContext` (emitted by ToolNode
|
||||
* for `read_file` tool calls in agents v3.1.72+) so the read lands in the
|
||||
* same session that holds the agent's prior-turn artifacts. Returns a
|
||||
* `ToolExecuteResult` with the file content (line-numbered) on success,
|
||||
* or an instructive error pointing the model at `bash_tool` when the
|
||||
* sandbox isn't reachable from this configuration.
|
||||
*/
|
||||
async function handleSandboxFileFallback(
|
||||
tc: ToolCallRequest,
|
||||
filePath: string,
|
||||
options: ToolExecuteOptions,
|
||||
): Promise<ToolExecuteResult> {
|
||||
const { readSandboxFile } = options;
|
||||
if (!readSandboxFile) {
|
||||
return {
|
||||
toolCallId: tc.id,
|
||||
status: 'error',
|
||||
content: '',
|
||||
errorMessage: `Path "${filePath}" is not a skill file. Use \`bash_tool\` to read code-execution sandbox files (e.g. \`cat ${filePath}\`).`,
|
||||
};
|
||||
}
|
||||
|
||||
const ctx = tc.codeSessionContext as
|
||||
| { session_id?: string; files?: Array<{ id: string; name: string; session_id?: string }> }
|
||||
| undefined;
|
||||
try {
|
||||
const result = await readSandboxFile({
|
||||
file_path: filePath,
|
||||
session_id: ctx?.session_id,
|
||||
files: ctx?.files,
|
||||
});
|
||||
if (!result || result.content == null) {
|
||||
return {
|
||||
toolCallId: tc.id,
|
||||
status: 'error',
|
||||
content: '',
|
||||
errorMessage: `Failed to read "${filePath}" from the code-execution sandbox. Try \`bash_tool\` (e.g. \`cat ${filePath}\`).`,
|
||||
};
|
||||
}
|
||||
return {
|
||||
toolCallId: tc.id,
|
||||
status: 'success',
|
||||
content: addLineNumbers(result.content),
|
||||
};
|
||||
} catch (error) {
|
||||
const message = error instanceof Error ? error.message : String(error);
|
||||
logger.warn(`[handleReadFileCall] Sandbox fallback failed for "${filePath}": ${message}`);
|
||||
return {
|
||||
toolCallId: tc.id,
|
||||
status: 'error',
|
||||
content: '',
|
||||
errorMessage: `Error reading "${filePath}" from the code-execution sandbox: ${message}. Try \`bash_tool\` (e.g. \`cat ${filePath}\`).`,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
async function handleReadFileCall(
|
||||
tc: ToolCallRequest,
|
||||
mergedConfigurable: Record<string, unknown>,
|
||||
|
|
@ -151,8 +224,39 @@ async function handleReadFileCall(
|
|||
};
|
||||
}
|
||||
|
||||
const codeEnvAvailable = mergedConfigurable?.codeEnvAvailable === true;
|
||||
const accessibleIds = (mergedConfigurable?.accessibleSkillIds as Types.ObjectId[]) ?? [];
|
||||
/**
|
||||
* `accessibleSkillIds` is the resolver's authoritative output (admin
|
||||
* capability AND ACL access AND ephemeral badge / persisted
|
||||
* `skills_enabled`). Empty ⇒ skills are not effectively in scope, so we
|
||||
* skip skill resolution entirely and route to the sandbox fallback when
|
||||
* the agent has code-execution available.
|
||||
*/
|
||||
const skillsEffectivelyEnabled = accessibleIds.length > 0;
|
||||
|
||||
/**
|
||||
* Short-circuit absolute code-env paths: the path can never be a skill
|
||||
* reference (skill paths are relative `{skillName}/...`), and consulting
|
||||
* `getSkillByName` would just burn a DB round-trip on a guaranteed miss.
|
||||
*/
|
||||
if (args.file_path.startsWith('/mnt/data/')) {
|
||||
if (codeEnvAvailable) {
|
||||
return handleSandboxFileFallback(tc, args.file_path, options);
|
||||
}
|
||||
return {
|
||||
toolCallId: tc.id,
|
||||
status: 'error',
|
||||
content: '',
|
||||
errorMessage: `Path "${args.file_path}" is a code-execution sandbox path, but this agent does not have code execution enabled.`,
|
||||
};
|
||||
}
|
||||
|
||||
const slashIdx = args.file_path.indexOf('/');
|
||||
if (slashIdx < 1) {
|
||||
if (codeEnvAvailable) {
|
||||
return handleSandboxFileFallback(tc, args.file_path, options);
|
||||
}
|
||||
return {
|
||||
toolCallId: tc.id,
|
||||
status: 'error',
|
||||
|
|
@ -172,6 +276,45 @@ async function handleReadFileCall(
|
|||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Skills not in scope (admin capability off, ephemeral badge off, or
|
||||
* persisted `skills_enabled !== true` — all already collapsed into
|
||||
* `accessibleSkillIds.length === 0` by `resolveAgentScopedSkillIds`):
|
||||
* route to the sandbox fallback when code execution is available, else
|
||||
* the lookup truly has nowhere to go.
|
||||
*/
|
||||
if (!skillsEffectivelyEnabled) {
|
||||
if (codeEnvAvailable) {
|
||||
return handleSandboxFileFallback(tc, args.file_path, options);
|
||||
}
|
||||
return {
|
||||
toolCallId: tc.id,
|
||||
status: 'error',
|
||||
content: '',
|
||||
errorMessage:
|
||||
'Skill files are not available for this agent and code execution is not enabled.',
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Skills are in scope, but the first segment isn't a name we know.
|
||||
* Use the catalog-derived `activeSkillNames` Set (no DB read) to detect
|
||||
* this and fall through to the sandbox so the model doesn't have to
|
||||
* eat a wasted `read_file` error before retrying with `bash_tool`.
|
||||
*/
|
||||
const activeSkillNames = mergedConfigurable?.activeSkillNames as Set<string> | undefined;
|
||||
if (activeSkillNames && !activeSkillNames.has(skillName)) {
|
||||
if (codeEnvAvailable) {
|
||||
return handleSandboxFileFallback(tc, args.file_path, options);
|
||||
}
|
||||
return {
|
||||
toolCallId: tc.id,
|
||||
status: 'error',
|
||||
content: '',
|
||||
errorMessage: `Skill "${skillName}" not found or not accessible`,
|
||||
};
|
||||
}
|
||||
|
||||
if (!getSkillByName) {
|
||||
return {
|
||||
toolCallId: tc.id,
|
||||
|
|
@ -181,7 +324,6 @@ async function handleReadFileCall(
|
|||
};
|
||||
}
|
||||
|
||||
const accessibleIds = (mergedConfigurable?.accessibleSkillIds as Types.ObjectId[]) ?? [];
|
||||
const skillPrimedIdsByName =
|
||||
(mergedConfigurable?.skillPrimedIdsByName as Record<string, string> | undefined) ?? {};
|
||||
const primedIdString = skillPrimedIdsByName[skillName];
|
||||
|
|
|
|||
|
|
@ -91,6 +91,13 @@ export type InitializedAgent = Agent & {
|
|||
codeEnvAvailable: boolean;
|
||||
/** Accessible skill IDs for ACL checking at execute time */
|
||||
accessibleSkillIds?: import('mongoose').Types.ObjectId[];
|
||||
/**
|
||||
* Names of skills the runtime can resolve, mirroring `accessibleSkillIds`.
|
||||
* Surfaced to the runtime so handlers like `read_file` can decide if a
|
||||
* `{firstSegment}/...` path refers to a real skill (vs. a code-env path
|
||||
* that should be routed to the bash fallback) without an extra DB lookup.
|
||||
*/
|
||||
activeSkillNames?: Set<string>;
|
||||
/** Number of skills in the catalog (used to determine if SkillTool should be registered) */
|
||||
skillCount?: number;
|
||||
/**
|
||||
|
|
@ -832,6 +839,7 @@ export async function initializeAgent(
|
|||
* LLM (or a direct-invocation path) names one.
|
||||
*/
|
||||
let executableSkillIds = params.accessibleSkillIds;
|
||||
let activeSkillNames: Set<string> | undefined;
|
||||
const { accessibleSkillIds } = params;
|
||||
if (accessibleSkillIds && accessibleSkillIds.length > 0) {
|
||||
const skillResult = await injectSkillCatalog({
|
||||
|
|
@ -849,6 +857,7 @@ export async function initializeAgent(
|
|||
toolDefinitions = skillResult.toolDefinitions;
|
||||
skillCount = skillResult.skillCount;
|
||||
executableSkillIds = skillResult.activeSkillIds;
|
||||
activeSkillNames = skillResult.activeSkillNames;
|
||||
}
|
||||
|
||||
const agentMaxContextNum = Number(agentMaxContextTokens) || DEFAULT_MAX_CONTEXT_TOKENS;
|
||||
|
|
@ -885,6 +894,7 @@ export async function initializeAgent(
|
|||
codeEnvAvailable: effectiveCodeEnvAvailable,
|
||||
skillCount,
|
||||
accessibleSkillIds: executableSkillIds,
|
||||
activeSkillNames,
|
||||
manualSkillPrimes,
|
||||
alwaysApplySkillPrimes,
|
||||
attachments: finalAttachments,
|
||||
|
|
|
|||
|
|
@ -35,6 +35,17 @@ export function enrichWithSkillConfigurable(
|
|||
* the exact doc the resolver primed and relaxes the disable-model gate.
|
||||
*/
|
||||
skillPrimedIdsByName?: Record<string, string>,
|
||||
/**
|
||||
* Names of skills the runtime can resolve, captured at agent init by
|
||||
* `injectSkillCatalog`. Lets `read_file` decide whether a
|
||||
* `{firstSegment}/...` path is a real skill reference vs. a code-env path
|
||||
* that should fall back to bash, without an extra `getSkillByName`
|
||||
* round-trip just to discover the name doesn't resolve. Empty/undefined
|
||||
* ⇒ skills aren't effectively in scope for this agent (admin capability
|
||||
* off, ephemeral badge off, or persisted `skills_enabled !== true`),
|
||||
* which is the same signal as `accessibleSkillIds.length === 0`.
|
||||
*/
|
||||
activeSkillNames?: Set<string>,
|
||||
): { loadedTools: unknown[]; configurable: Record<string, unknown> } {
|
||||
return {
|
||||
...result,
|
||||
|
|
@ -44,6 +55,7 @@ export function enrichWithSkillConfigurable(
|
|||
codeEnvAvailable,
|
||||
accessibleSkillIds,
|
||||
skillPrimedIdsByName,
|
||||
activeSkillNames,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -231,6 +231,15 @@ export interface InjectSkillCatalogResult {
|
|||
* resolvable.
|
||||
*/
|
||||
activeSkillIds: Types.ObjectId[];
|
||||
/**
|
||||
* Names of skills the runtime can resolve, mirroring `activeSkillIds`.
|
||||
* Surfaced so host-side handlers (e.g. `read_file`) can decide whether a
|
||||
* `{firstSegment}/...` path is a real skill reference vs. a code-env path
|
||||
* (`/mnt/data/...`) that should be routed to the bash fallback — without
|
||||
* issuing an extra `getSkillByName` round-trip just to discover the name
|
||||
* doesn't resolve.
|
||||
*/
|
||||
activeSkillNames: Set<string>;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -261,7 +270,12 @@ export async function injectSkillCatalog(
|
|||
} = params;
|
||||
|
||||
if (!listSkillsByAccess || accessibleSkillIds.length === 0) {
|
||||
return { toolDefinitions: inputDefs, skillCount: 0, activeSkillIds: [] };
|
||||
return {
|
||||
toolDefinitions: inputDefs,
|
||||
skillCount: 0,
|
||||
activeSkillIds: [],
|
||||
activeSkillNames: new Set<string>(),
|
||||
};
|
||||
}
|
||||
|
||||
type SkillSummary = Awaited<ReturnType<NonNullable<typeof listSkillsByAccess>>>['skills'][number];
|
||||
|
|
@ -321,7 +335,12 @@ export async function injectSkillCatalog(
|
|||
}
|
||||
|
||||
if (activeSkills.length === 0) {
|
||||
return { toolDefinitions: inputDefs, skillCount: 0, activeSkillIds: [] };
|
||||
return {
|
||||
toolDefinitions: inputDefs,
|
||||
skillCount: 0,
|
||||
activeSkillIds: [],
|
||||
activeSkillNames: new Set<string>(),
|
||||
};
|
||||
}
|
||||
|
||||
if (!reachedEnd && visibleCount < SKILL_CATALOG_LIMIT) {
|
||||
|
|
@ -442,6 +461,7 @@ export async function injectSkillCatalog(
|
|||
toolDefinitions: workingDefs,
|
||||
skillCount: catalogVisibleSkills.length,
|
||||
activeSkillIds: executableSkills.map((s) => s._id),
|
||||
activeSkillNames: new Set<string>(executableSkills.map((s) => s.name)),
|
||||
};
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue