mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 14:57:42 +00:00
🌱 fix: Inject Code-Tool Files Into Graph Sessions on First Call (+ read_file Sandbox Fallback) (#12831)
* 🌱 fix: Seed Code Tool Files Into Graph Sessions on First Call
Files attached to an agent's `tool_resources.execute_code` (user uploads
or generated artifacts from a prior turn) were silently dropped on the
first `execute_code` invocation of a turn. The agents-side `ToolNode`
populates `_injected_files` only when its `sessions` map already has an
`EXECUTE_CODE` entry — but that entry is only written by a previous
successful execution, so call #1 had nothing to inject. CodeExecutor
then fell back to a `/files/{session_id}` fetch, but `session_id` was
also empty on call #1, leaving the sandbox without the primed files.
Mirror the existing skill-priming pattern (`primeInvokedSkills` →
`initialSessions`) for code-resource files: eagerly call `primeFiles`
before `createRun` and merge the result into `initialSessions` via a
new `seedCodeFilesIntoSessions` helper. Skill files and code-resource
files now share the same `EXECUTE_CODE` entry; the prior representative
`session_id` is preserved on merge.
* 🔬 chore: Add Diagnostic Logging for Code-Files Seeding
Temporary debug logs to diagnose why first-call file injection is not
firing in real agent runs. Logs `wantsCodeExec`, available tool-resource
keys, primed file count, and the seeded EXECUTE_CODE entry. Will revert
once the failure mode is identified.
* 🪛 refactor: Capture primedCodeFiles per-agent at init, merge across run
Replace the client.js eager `primeFiles` call with a per-agent capture at
initialization time so every agent in a multi-agent run (primary +
handoff + addedConvo) contributes its `tool_resources.execute_code`
files to the shared `Graph.sessions` seed.
- handleTools.js (eager loadTools): the `execute_code` factory closes
over a `primedCodeFiles` slot and surfaces it in the return.
- ToolService.js loadToolDefinitionsWrapper (event-driven): captures
`files` from the existing `primeCodeFiles` call (was dropping them
while only keeping `toolContext`) and surfaces them.
- packages/api initialize.ts: the loadTools callback contract now
includes `primedCodeFiles`, threaded onto `InitializedAgent`.
- client.js: iterate `[primary, ...agentConfigs.values()]` and merge
each agent's `primedCodeFiles` into `initialSessions`. Drop the
primary-only `primeCodeFiles` call and diagnostic logs from the prior
attempt — wrong layer (single-agent), wrong gate (`agent.tools`
contained Tool instances after init, so the `.includes("execute_code")`
string check always failed).
* 🔬 chore: Add per-agent diagnostic logs for code-files seeding
Logs `tool_resources` keys + file counts inside loadToolDefinitionsWrapper
and per-agent `primedCodeFiles` + final initialSessions inside
AgentClient. Will revert once the failure mode is confirmed.
* 🔬 chore: Add file-lookup diagnostics inside initializeAgent
Logs the inputs and intermediate counts of the conversation-file lookup
chain (convo file ids, thread message ids, code-generated and
user-code file counts) so we can pinpoint why `tool_resources.execute_code`
is arriving empty at `loadToolDefinitionsWrapper` despite the agent
having `execute_code` in its tools list.
* 🔬 chore: Probe execute_code files without messageId filter
Adds a relaxed `getFiles({conversationId, context: execute_code})` probe
that runs only when `getCodeGeneratedFiles` returns empty. Lists what's
actually in the DB for this conversation so we can confirm whether the
file is missing entirely or whether the messageId filter is rejecting it.
* 🔬 chore: Fix probe getFiles arg order (sort vs projection)
Probe was passing a projection object as the sort arg, which mongoose
rejected with `Invalid sort value`. Move it to the third arg
(selectFields) so the probe actually runs.
* 🪢 fix: Preserve Original messageId on Code-Output File Update
Each `processCodeOutput` call was overwriting the persisted file's
`messageId` with the *current* run's id. When a turn re-creates an
existing file (filename + conversationId match → `claimCodeFile`
returns the existing record, `isUpdate=true`), the file's link to
the assistant message that originally produced it gets clobbered.
`initializeAgent` later runs `getCodeGeneratedFiles({messageId: $in: <thread>})`
to seed `tool_resources.execute_code` from prior-turn artifacts. With a
stale `messageId` (e.g. from a failed read attempt that re-shelled the
same filename), the file no longer matches the parent-walk thread, so
`tool_resources` arrives empty at agent init, the new
`primedCodeFiles` channel has nothing to seed, and the LLM can't see
its own prior-turn artifacts on the next turn — defeating the
just-added Graph-sessions seeding fix.
Preserve the existing `claimed.messageId` on update; first-creation
behavior is unchanged. The runtime return value still includes the
current run's `messageId` (via `Object.assign(file, { messageId })`)
so the artifact is correctly attributed to the live tool_call.
* 🧹 chore: Remove diagnostic logs from code-files seeding path
Drops the temporary debug logs added to trace the empty-tool_resources
failure mode. Production code paths (loadToolDefinitionsWrapper,
client.js seed loop, initializeAgent file lookup) are left as the
permanent shape: capture primedCodeFiles, merge across agents, seed
initialSessions before run start.
* 🪛 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`).
* 🔧 chore: Update @librechat/agents dependency to version 3.1.72
Bumps the version of the @librechat/agents package across package-lock.json and relevant package.json files to ensure compatibility with the latest features and fixes.
* 🪛 refactor: Centralize Tool-Session Seed in buildInitialToolSessions Helper
Addresses review feedback on the per-agent merge in client.js:
- **Run-wide semantics, named explicitly.** The merge into a single
`Graph.sessions[EXECUTE_CODE]` was a deliberate match to the
agents-library design (`Graph.sessions` is shared across every
`ToolNode` in the run), but the inline `for (const a of agents)`
loop in `AgentClient.chatCompletion` made it look per-agent. Move
the logic to a TS helper `buildInitialToolSessions` that documents
the run-wide-by-design contract in one place. The CJS controller
now contains a single call site, no business logic.
- **Subagent walk (P2).** The previous loop only iterated
`[primary, ...agentConfigs.values()]`. Pure subagents are pruned
out of `agentConfigs` after init and retained on each parent's
`subagentAgentConfigs`, so their primed code files were silently
dropped from the seed. The helper now walks recursively, with a
visited-Set keyed on object identity that terminates safely on a
malformed agent graph (cycle).
- **`jest.setup.cjs` polyfill for undici `File`.** Reviewer hit
`ReferenceError: File is not defined` running the targeted spec on
WSL — a known Node 18 issue where `globalThis.File` from
`node:buffer` isn't auto-exposed. Polyfill it inside a Jest setup
file so the suite boots regardless of Node patch version.
Helper test coverage (8 new): skill-only / agent-only / both,
recursive subagent walk, cycle-safe walk, primary+subagent
deduplication, undefined/null entries in the agents iterable, and
representative session_id preservation across the merge.
16 tests pass total in `codeFilesSession.spec.ts` (8 prior + 8 new).
No behavior change vs. the previous commit for the existing
primary+agentConfigs case — subagent inclusion is the only new
behavior, and it matches what the existing seeding logic would have
done if subagents had been in `agentConfigs`.
* 🪛 fix: FIFO Walk Order in buildInitialToolSessions (P3 review)
The traversal used `Array.pop()` (LIFO), which visited the LAST
top-level agent first. The docstring says "primary first"; the code
contradicted it. When no skill seed exists the first-visited agent's
first file supplies the representative `session_id` written to
`Graph.sessions[EXECUTE_CODE]` — so a LIFO walk silently flipped which
agent that came from. `ToolNode` ultimately uses per-file `session_id`s
for runtime injection (so behavior was indistinguishable for current
callers), but the discrepancy was a footgun for any future consumer
that read the representative.
Switch to FIFO via `Array.shift()` to match both the docstring and the
existing `loadSubagentsFor` walk pattern in
`Endpoints/agents/initialize.js`. Add a regression test that asserts
the primary's `session_id` is the representative (and that all three
agents' files still contribute, with per-file `session_id`s preserved).
* 🔬 test: Lock In Code-Files Bug Fixes Per Comprehensive Review
Addresses MAJOR + MINOR + NIT findings from the multi-pass review:
**Finding #4 (MINOR) — empty relativePath misses sandbox fallback.**
A model calling `read_file("output/")` where "output" isn't a skill
name dead-ended with `Missing file path after skill name` instead of
being routed to the sandbox like every other malformed-path branch.
Add the same `codeEnvAvailable → handleSandboxFileFallback` pattern,
plus two regression tests.
**Finding #7 (NIT) — duplicate `skillsInScope()` helper.**
Hoist the identical helper out of two nested describe blocks to
module scope. Single source of truth.
**Finding #1 (MAJOR) — `persistedMessageId` had zero test coverage.**
The fix preserves a file's original `messageId` on update so
`getCodeGeneratedFiles` can still match it on subsequent turns. A
regression in the `isUpdate ? (claimed.messageId ?? messageId) : messageId`
ternary would silently re-introduce the original cross-turn priming
bug. Five new tests cover:
- UPDATE preserves `claimed.messageId` in the persisted record
- UPDATE falls back to current run id when `claimed.messageId` is
absent (legacy records predating the field)
- CREATE uses current run id (no claimed record exists)
- The runtime return value uses the LIVE id (artifact attribution)
even when the persisted record kept the original
- The image branch follows the same contract (would silently regress
if the ternary diverged across the two file-build branches)
The tests use a `snapshotCreateFileArgs()` helper because
`processCodeOutput` mutates the file object after `createFile`
returns (`Object.assign(file, { messageId, toolCallId })`) and a
naive `createFile.mock.calls[0][0]` would reflect the post-mutation
state instead of what was actually persisted.
**Finding #2 (MAJOR) — `readSandboxFile` had no direct tests.**
The model-controlled `file_path` flows through a POSIX single-quote
escape into a shell `cat` command, making this a security boundary.
A quoting regression would let a malicious filename break out of the
quoted argument and inject arbitrary shell. 20 new tests across:
- Shell quoting (7): plain filenames, embedded `'`, `$()`, backticks,
newlines, shell metachars, multiple consecutive single-quotes
- Payload shape (6): /exec URL, bash language, conditional
session_id / files inclusion, dedicated keepAlive:false agents
- Response handling (6): `{content}` on success, null on missing
base URL or absent stdout, throws on stderr-only, partial-success
returns stdout, transport errors are logged then rethrown
- Timeout (1): matches processCodeOutput's 15s SLA
Audited findings #5 (acknowledged tech debt — readSandboxFile in JS
workspace), #6 (pre-existing positional-args debt on
enrichWithSkillConfigurable), and #8 (cosmetic JSDoc style) — no
action taken per the reviewer's own assessment.
Audited finding #3 (walk order vs docstring) — already addressed in
commit 007f32341 which converted to FIFO via `queue.shift()` plus a
regression test. The audit was performed against an earlier PR head.
Tests: 152 packages/api + 195 api JS = 347 pass. Typecheck clean.
* 🪛 fix: Pure-Subagent codeEnv + Primed-Skill Routing + ToolService Early Returns
Three findings from the second-pass review:
**P2 — Pure subagents missed `codeEnvAvailable`** (initialize.js).
The pure-subagent init path didn't forward the endpoint-level
`codeEnvAvailable` flag to `initializeAgent`, unlike the primary,
handoff, and addedConvo paths. A code-enabled subagent loaded only
through `subagentAgentConfigs` initialized with
`codeEnvAvailable: false`, so even though the recursive seed walk
found its primed code files, the subagent's own `bash_tool` /
`read_file` sandbox fallback were silently gated off. Forward the
flag and add `codeEnvAvailable: config.codeEnvAvailable` to the
`agentToolContexts.set` for symmetry with the other paths.
**P2 — Primed skills outside the catalog cap were misrouted to
sandbox** (handlers.ts). Manual ($-popover) and always-apply primes
are intentionally resolved off the wider `accessibleSkillIds` ACL
set BEFORE catalog injection — see `resolveManualSkills` for why a
skill outside the `SKILL_CATALOG_LIMIT` cap can still be authorized
for direct manual invocation. The `activeSkillNames` shortcut ran
before reading `skillPrimedIdsByName`, so a primed skill not in the
catalog would fall through to the sandbox instead of resolving via
the pinned `_id`. Read the primed map first and bypass the shortcut
for primed names. New regression test asserts a primed-but-not-
cataloged skill resolves through the existing skill path with
`getSkillByName` invoked and `readSandboxFile` NOT called.
**P3 — `loadAgentTools` early returns dropped `primedCodeFiles`**
(ToolService.js). The non-`definitionsOnly` path captures the field
correctly, but two early-return branches (no-action-tools fast path,
no-action-sets fast path) omitted it. Any traditional
`loadAgentTools(..., definitionsOnly: false)` caller using
execute_code without action tools would have its first-call session
seed silently empty. Add `primedCodeFiles` to both early returns
for consistency with the final return shape.
Tests: 153 packages/api + 195 api JS = 348 pass.
* 🧹 chore: Document jest.mock arrow-indirection pattern in process.spec.js
Per the second-pass review's Finding #2 (NIT, "would help future
readers"): the mock setup mixes direct `jest.fn()` references with
arrow-function indirection (`(...args) => mockX(...args)`). The
indirection isn't stylistic — it's required because `jest.mock(...)`
is hoisted above the outer `const` declarations at parse time, so a
direct reference would capture `undefined`. Inline comment explains
the pattern so the next reader doesn't have to reverse-engineer it
or accidentally "simplify" the mocks and break per-test
`mockReturnValueOnce` / `mockImplementationOnce` overrides.
* 🪛 fix: Five Issues from Pass-N + Codex Review (incl. 404 root cause)
Five real bugs surfaced by another review pass + Codex PR comments
+ the codeapi-side logs we collected during manual testing:
**1) `processCodeOutput` 404 root cause (`callbacks.js`).**
The codeapi worker emits TWO distinct `session_id`s on a tool result:
- `artifact.session_id` is the EXEC session — the sandbox VM that
ran the bash command. Files don't live there; it's torn down
post-execution.
- `file.session_id` is the STORAGE session — the file-server
bucket prefix where artifacts actually live.
`callbacks.js` was passing the EXEC id to `processCodeOutput`, which
builds `/download/{session_id}/{id}` and 404s because the file-server
doesn't know about that path. This explains every "Error
downloading/processing code environment file" we saw during testing.
Use `file.session_id ?? output.artifact.session_id` (per-file id with
artifact-level fallback for older worker payloads).
**2) `primeFiles` reupload pushed STALE sandbox ids (`process.js`).**
When `getSessionInfo` returns null (file expired/missing in sandbox),
`reuploadFile` re-uploads via `handleFileUpload`, gets a NEW
`fileIdentifier`, and persists it on the DB record. But `pushFile`
was a closure capturing the OLD `(session_id, id)` parsed at the top
of the loop, so the in-memory `files[]` array (now consumed by
`buildInitialToolSessions` to seed `Graph.sessions`) silently
referenced a sandbox object that no longer existed. The first tool
call would 404 trying to mount it; only the next turn's metadata
re-read would correct course. Parameterize `pushFile` with optional
`(session_id, id)` overrides; in `reuploadFile` parse the new
identifier and pass through. 2 regression tests.
**3) Codex P2 — Cap sandbox fallback output before line-numbering
(`handlers.ts`).** The new `handleSandboxFileFallback` returned
`addLineNumbers(result.content)` without a size guard, so reading a
multi-MB `/mnt/data/*` artifact materialized the file twice in
memory (raw + line-numbered) before downstream truncation. Match the
existing skill-file path's `MAX_READABLE_BYTES` (256KB): truncate
raw first, then number, surface the truncation to the model so it
can use `bash_tool` (`head` / `tail`) for the rest. 2 tests
(oversized truncates with hint, in-cap doesn't).
**4) Codex P2 — Dedupe seeded code files by `(session_id, id)`
(`codeFilesSession.ts`).** Multiple agents in a run commonly carry
the same primed execute-code resources (shared conversation files);
without dedupe, `_injected_files` grows proportionally to agent
count and bloats every `/exec` POST. Use a `(session_id, id)`
identity key so first-seen wins (preserves source ordering); name
alone isn't sufficient because two distinct primed uploads can
share a filename across different sessions. 4 tests covering dedup
across iterations, against pre-existing entries, name-collision
distinct-session preservation, and the multi-agent realistic case
in `buildInitialToolSessions`.
**5) Pass-N P2 — Polyfill `globalThis.File` in api Jest setup
(`api/test/jestSetup.js`).** `packages/api/jest.setup.cjs` had the
polyfill; the legacy api workspace's Jest config has its own
`setupFiles` that didn't, so on Node 18 / WSL the api focused tests
still failed at import time with `ReferenceError: File is not
defined` from undici. Mirror the polyfill.
Tests: 159 packages/api + 206 api JS = 365 pass. Typecheck clean.
* 🔧 chore: Update @librechat/agents dependency to version 3.1.73
Bumps the version of the @librechat/agents package across package-lock.json and relevant package.json files to ensure compatibility with the latest features and fixes.
This commit is contained in:
parent
f7e47f6012
commit
24e29aa8cb
22 changed files with 1923 additions and 33 deletions
|
|
@ -249,6 +249,15 @@ const loadTools = async ({
|
|||
|
||||
/** @type {Record<string, string>} */
|
||||
const toolContextMap = {};
|
||||
/**
|
||||
* @type {import('@librechat/agents').CodeEnvFile[] | undefined}
|
||||
* Captured by the `execute_code` factory when files are primed. Surfaced
|
||||
* out of `loadTools` so client.js can seed `Graph.sessions[EXECUTE_CODE]`
|
||||
* before run start — without that seed, the first `execute_code` /
|
||||
* `bash_tool` call lands with empty `_injected_files` and the sandbox
|
||||
* can't see the prior turn's generated artifacts.
|
||||
*/
|
||||
let primedCodeFiles;
|
||||
const requestedMCPTools = {};
|
||||
|
||||
/** Resolve config-source servers for the current user/tenant context */
|
||||
|
|
@ -267,6 +276,9 @@ const loadTools = async ({
|
|||
if (toolContext) {
|
||||
toolContextMap[tool] = toolContext;
|
||||
}
|
||||
if (files?.length) {
|
||||
primedCodeFiles = files;
|
||||
}
|
||||
return createCodeExecutionTool({ user_id: user, files });
|
||||
};
|
||||
continue;
|
||||
|
|
@ -474,7 +486,7 @@ const loadTools = async ({
|
|||
}
|
||||
}
|
||||
loadedTools.push(...(await Promise.all(mcpToolPromises)).flatMap((plugin) => plugin || []));
|
||||
return { loadedTools, toolContextMap };
|
||||
return { loadedTools, toolContextMap, primedCodeFiles };
|
||||
};
|
||||
|
||||
module.exports = {
|
||||
|
|
|
|||
|
|
@ -44,7 +44,7 @@
|
|||
"@google/genai": "^1.19.0",
|
||||
"@keyv/redis": "^4.3.3",
|
||||
"@langchain/core": "^0.3.80",
|
||||
"@librechat/agents": "^3.1.71",
|
||||
"@librechat/agents": "^3.1.73",
|
||||
"@librechat/api": "*",
|
||||
"@librechat/data-schemas": "*",
|
||||
"@microsoft/microsoft-graph-client": "^3.0.7",
|
||||
|
|
|
|||
|
|
@ -553,7 +553,23 @@ function createToolEndCallback({ req, res, artifactPromises, streamId = null })
|
|||
messageId: metadata.run_id,
|
||||
toolCallId: output.tool_call_id,
|
||||
conversationId: metadata.thread_id,
|
||||
session_id: output.artifact.session_id,
|
||||
/**
|
||||
* Use the FILE's session_id (storage session), not the
|
||||
* top-level artifact session_id (exec session). The codeapi
|
||||
* worker reports two distinct ids on a tool result:
|
||||
* - `artifact.session_id` is the EXEC session — the
|
||||
* sandbox VM that ran the bash command. Files don't
|
||||
* live there; it's torn down post-execution.
|
||||
* - `file.session_id` is the STORAGE session — the
|
||||
* file-server bucket prefix where artifacts actually
|
||||
* live and are served from.
|
||||
* `processCodeOutput` builds `/download/{session_id}/{id}`,
|
||||
* so passing the exec id resolves to a path the file-server
|
||||
* doesn't know about and 404s. Fall back to artifact-level
|
||||
* for older worker payloads that may not populate per-file
|
||||
* ids.
|
||||
*/
|
||||
session_id: file.session_id ?? output.artifact.session_id,
|
||||
});
|
||||
if (!streamId && !res.headersSent) {
|
||||
return fileMetadata;
|
||||
|
|
@ -754,7 +770,23 @@ function createResponsesToolEndCallback({ req, res, tracker, artifactPromises })
|
|||
messageId: metadata.run_id,
|
||||
toolCallId: output.tool_call_id,
|
||||
conversationId: metadata.thread_id,
|
||||
session_id: output.artifact.session_id,
|
||||
/**
|
||||
* Use the FILE's session_id (storage session), not the
|
||||
* top-level artifact session_id (exec session). The codeapi
|
||||
* worker reports two distinct ids on a tool result:
|
||||
* - `artifact.session_id` is the EXEC session — the
|
||||
* sandbox VM that ran the bash command. Files don't
|
||||
* live there; it's torn down post-execution.
|
||||
* - `file.session_id` is the STORAGE session — the
|
||||
* file-server bucket prefix where artifacts actually
|
||||
* live and are served from.
|
||||
* `processCodeOutput` builds `/download/{session_id}/{id}`,
|
||||
* so passing the exec id resolves to a path the file-server
|
||||
* doesn't know about and 404s. Fall back to artifact-level
|
||||
* for older worker payloads that may not populate per-file
|
||||
* ids.
|
||||
*/
|
||||
session_id: file.session_id ?? output.artifact.session_id,
|
||||
});
|
||||
|
||||
if (!fileMetadata) {
|
||||
|
|
|
|||
|
|
@ -31,6 +31,7 @@ const {
|
|||
injectSkillPrimes,
|
||||
isSkillPrimeMessage,
|
||||
buildSkillPrimeContentParts,
|
||||
buildInitialToolSessions,
|
||||
} = require('@librechat/api');
|
||||
const {
|
||||
Callback,
|
||||
|
|
@ -815,6 +816,18 @@ class AgentClient extends BaseClient {
|
|||
? await this.options.primeInvokedSkills(payload)
|
||||
: undefined;
|
||||
|
||||
/**
|
||||
* Seed `Graph.sessions` with code-env files primed across every
|
||||
* reachable agent (primary, handoff/addedConvo, and nested
|
||||
* subagents) plus skill-priming output. The merge logic and its
|
||||
* run-wide semantics live in `buildInitialToolSessions`; see that
|
||||
* helper's doc for why this is intentionally NOT per-agent.
|
||||
*/
|
||||
const initialSessions = buildInitialToolSessions({
|
||||
skillSessions: skillPrimeResult?.initialSessions,
|
||||
agents: [this.options.agent, ...(this.agentConfigs ? this.agentConfigs.values() : [])],
|
||||
});
|
||||
|
||||
let {
|
||||
messages: initialMessages,
|
||||
indexTokenCountMap,
|
||||
|
|
@ -943,7 +956,7 @@ class AgentClient extends BaseClient {
|
|||
messages,
|
||||
indexTokenCountMap,
|
||||
initialSummary,
|
||||
initialSessions: skillPrimeResult?.initialSessions,
|
||||
initialSessions,
|
||||
calibrationRatio,
|
||||
runId: this.responseMessageId,
|
||||
signal: abortController.signal,
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
});
|
||||
}
|
||||
|
|
@ -562,6 +566,15 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => {
|
|||
skillsCapabilityEnabled,
|
||||
ephemeralSkillsToggle,
|
||||
}),
|
||||
/** Match the primary / handoff / addedConvo paths: forward the
|
||||
* endpoint-level admin flag so `initializeAgent` can compute the
|
||||
* per-agent narrowing (admin AND agent.tools includes
|
||||
* execute_code) into `InitializedAgent.codeEnvAvailable`. Without
|
||||
* this, a code-enabled subagent loaded only through
|
||||
* `subagentAgentConfigs` initializes with `codeEnvAvailable:
|
||||
* false`, so `bash_tool` / `read_file` sandbox fallback are
|
||||
* silently gated off even though the seed walk found it. */
|
||||
codeEnvAvailable,
|
||||
skillStates,
|
||||
defaultActiveOnShare,
|
||||
},
|
||||
|
|
@ -594,6 +607,8 @@ 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,
|
||||
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() {
|
||||
|
|
|
|||
|
|
@ -162,6 +162,16 @@ const processCodeOutput = async ({
|
|||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Preserve the original `messageId` on update. Each `processCodeOutput`
|
||||
* call would otherwise overwrite it with the current run's run id, which
|
||||
* decouples the file from the assistant message that originally created
|
||||
* it. `getCodeGeneratedFiles` filters by `messageId IN <thread>`, so a
|
||||
* stale id (e.g. from a later regeneration / failed re-read attempt)
|
||||
* silently excludes the file from priming on subsequent turns.
|
||||
*/
|
||||
const persistedMessageId = isUpdate ? (claimed.messageId ?? messageId) : messageId;
|
||||
|
||||
if (isImage) {
|
||||
const usage = isUpdate ? (claimed.usage ?? 0) + 1 : 1;
|
||||
const _file = await convertImage(req, buffer, 'high', `${file_id}${fileExt}`);
|
||||
|
|
@ -170,7 +180,7 @@ const processCodeOutput = async ({
|
|||
..._file,
|
||||
filepath,
|
||||
file_id,
|
||||
messageId,
|
||||
messageId: persistedMessageId,
|
||||
usage,
|
||||
filename: safeName,
|
||||
conversationId,
|
||||
|
|
@ -230,7 +240,7 @@ const processCodeOutput = async ({
|
|||
const file = {
|
||||
file_id,
|
||||
filepath,
|
||||
messageId,
|
||||
messageId: persistedMessageId,
|
||||
object: 'file',
|
||||
filename: safeName,
|
||||
type: mimeType,
|
||||
|
|
@ -374,7 +384,19 @@ const primeFiles = async (options) => {
|
|||
const [path, queryString] = file.metadata.fileIdentifier.split('?');
|
||||
const [session_id, id] = path.split('/');
|
||||
|
||||
const pushFile = () => {
|
||||
/**
|
||||
* `pushFile` accepts optional overrides so the reupload path can
|
||||
* push the FRESH `(session_id, id)` parsed off the new
|
||||
* `fileIdentifier`. Without these overrides, the closure would
|
||||
* capture the stale pre-reupload refs from the outer loop and
|
||||
* the in-memory `files` array (now consumed by
|
||||
* `buildInitialToolSessions` to seed `Graph.sessions`) would
|
||||
* point at a sandbox object that no longer exists. The DB record
|
||||
* gets the new identifier via `updateFile`, but the seed would
|
||||
* still inject the old one — bash_tool / read_file would 404
|
||||
* trying to mount the file until the next turn re-reads metadata.
|
||||
*/
|
||||
const pushFile = (overrideSessionId, overrideId) => {
|
||||
if (!toolContext) {
|
||||
toolContext = `- Note: The following files are available in the "${Tools.execute_code}" tool environment:`;
|
||||
}
|
||||
|
|
@ -389,8 +411,8 @@ const primeFiles = async (options) => {
|
|||
|
||||
toolContext += `\n\t- /mnt/data/${file.filename}${fileSuffix}`;
|
||||
files.push({
|
||||
id,
|
||||
session_id,
|
||||
id: overrideId ?? id,
|
||||
session_id: overrideSessionId ?? session_id,
|
||||
name: file.filename,
|
||||
});
|
||||
};
|
||||
|
|
@ -429,8 +451,18 @@ const primeFiles = async (options) => {
|
|||
file_id: file.file_id,
|
||||
metadata: updatedMetadata,
|
||||
});
|
||||
sessions.set(session_id, true);
|
||||
pushFile();
|
||||
/**
|
||||
* Parse the FRESH fileIdentifier returned by the reupload and
|
||||
* route it through both the dedupe Map and the in-memory
|
||||
* `files` list. The original `(session_id, id)` parsed at the
|
||||
* top of this iteration refer to the old, expired/missing
|
||||
* sandbox object — using them here would silently re-introduce
|
||||
* the bug `Graph.sessions` seeding is supposed to fix.
|
||||
*/
|
||||
const [newPath] = fileIdentifier.split('?');
|
||||
const [newSessionId, newId] = newPath.split('/');
|
||||
sessions.set(newSessionId, true);
|
||||
pushFile(newSessionId, newId);
|
||||
} catch (error) {
|
||||
logger.error(
|
||||
`Error re-uploading file ${id} in session ${session_id}: ${error.message}`,
|
||||
|
|
@ -456,9 +488,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,
|
||||
};
|
||||
|
|
|
|||
|
|
@ -51,6 +51,15 @@ jest.mock('@librechat/api', () => {
|
|||
getBasePath: jest.fn(() => ''),
|
||||
sanitizeFilename: jest.fn((name) => name),
|
||||
createAxiosInstance: jest.fn(() => mockAxios),
|
||||
/**
|
||||
* Arrow-function indirection (vs. a direct `jest.fn()` reference) so
|
||||
* tests can per-case `mockReturnValueOnce` / `mockImplementationOnce`
|
||||
* on `mockClassifyCodeArtifact` / `mockExtractCodeArtifactText`.
|
||||
* `jest.mock(...)` is hoisted above the outer `const` declarations
|
||||
* at parse time, so a direct reference here would capture
|
||||
* `undefined`; the arrow defers the binding to call time. The
|
||||
* direct-`jest.fn()` mocks below stay constant per file.
|
||||
*/
|
||||
classifyCodeArtifact: (...args) => mockClassifyCodeArtifact(...args),
|
||||
extractCodeArtifactText: (...args) => mockExtractCodeArtifactText(...args),
|
||||
codeServerHttpAgent: new http.Agent({ keepAlive: false }),
|
||||
|
|
@ -108,7 +117,7 @@ const { determineFileType } = require('~/server/utils');
|
|||
const { logger } = require('@librechat/data-schemas');
|
||||
const { codeServerHttpAgent, codeServerHttpsAgent } = require('@librechat/api');
|
||||
|
||||
const { processCodeOutput, getSessionInfo } = require('./process');
|
||||
const { processCodeOutput, getSessionInfo, readSandboxFile, primeFiles } = require('./process');
|
||||
|
||||
describe('Code Process', () => {
|
||||
const mockReq = {
|
||||
|
|
@ -492,6 +501,165 @@ describe('Code Process', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('persistedMessageId (regression for cross-turn priming)', () => {
|
||||
/**
|
||||
* `getCodeGeneratedFiles` filters by `messageId IN <thread message ids>`
|
||||
* to scope files to the current branch. If `processCodeOutput` overwrote
|
||||
* the file's `messageId` with the current run's id on every update, a
|
||||
* file re-touched by a later turn (e.g. a failed read attempt that
|
||||
* re-shells the same filename) would lose its link to the assistant
|
||||
* message that originally produced it. Subsequent turns then can't find
|
||||
* it via `getCodeGeneratedFiles`, the priming chain has nothing to seed,
|
||||
* and the model thinks its own prior-turn artifact disappeared.
|
||||
*
|
||||
* Contract:
|
||||
* - On UPDATE (claimCodeFile returned an existing record): the persisted
|
||||
* `messageId` is `claimed.messageId` (preserved). Falls back to the
|
||||
* current run's `messageId` when the existing record predates the
|
||||
* `messageId` field (legacy data).
|
||||
* - On CREATE (new file): the persisted `messageId` is the current run's.
|
||||
* - The runtime return value ALWAYS uses the current run's `messageId`
|
||||
* via `Object.assign(file, { messageId, toolCallId })` so the artifact
|
||||
* attaches to the correct tool_call in the live response.
|
||||
*/
|
||||
|
||||
/**
|
||||
* `processCodeOutput` mutates the file object after `createFile` returns
|
||||
* (`Object.assign(file, { messageId, toolCallId })`) so the runtime
|
||||
* caller sees the live messageId on the response. Reading
|
||||
* `createFile.mock.calls[0][0]` directly would therefore reflect the
|
||||
* post-mutation state because JS captures by reference. To assert
|
||||
* what was actually PERSISTED, snapshot the args at call time.
|
||||
*/
|
||||
function snapshotCreateFileArgs() {
|
||||
const snapshots = [];
|
||||
createFile.mockImplementation(async (file) => {
|
||||
snapshots.push({ ...file });
|
||||
return {};
|
||||
});
|
||||
return snapshots;
|
||||
}
|
||||
|
||||
it('preserves the original messageId in the persisted record on UPDATE', async () => {
|
||||
mockClaimCodeFile.mockResolvedValue({
|
||||
file_id: 'existing-id',
|
||||
filename: 'sentinel.txt',
|
||||
usage: 1,
|
||||
createdAt: '2024-01-01T00:00:00.000Z',
|
||||
messageId: 'turn-1-original-msg',
|
||||
});
|
||||
const persisted = snapshotCreateFileArgs();
|
||||
|
||||
const smallBuffer = Buffer.alloc(100);
|
||||
mockAxios.mockResolvedValue({ data: smallBuffer });
|
||||
|
||||
await processCodeOutput({
|
||||
...baseParams,
|
||||
name: 'sentinel.txt',
|
||||
messageId: 'turn-2-current-run-msg',
|
||||
});
|
||||
|
||||
expect(persisted[0].messageId).toBe('turn-1-original-msg');
|
||||
});
|
||||
|
||||
it('falls back to current run messageId on UPDATE when claimed.messageId is undefined (legacy record)', async () => {
|
||||
// Legacy record predates the persistedMessageId tracking.
|
||||
mockClaimCodeFile.mockResolvedValue({
|
||||
file_id: 'legacy-id',
|
||||
filename: 'legacy.txt',
|
||||
usage: 1,
|
||||
createdAt: '2024-01-01T00:00:00.000Z',
|
||||
// messageId intentionally absent
|
||||
});
|
||||
const persisted = snapshotCreateFileArgs();
|
||||
|
||||
const smallBuffer = Buffer.alloc(100);
|
||||
mockAxios.mockResolvedValue({ data: smallBuffer });
|
||||
|
||||
await processCodeOutput({
|
||||
...baseParams,
|
||||
name: 'legacy.txt',
|
||||
messageId: 'turn-N-current-run-msg',
|
||||
});
|
||||
|
||||
expect(persisted[0].messageId).toBe('turn-N-current-run-msg');
|
||||
});
|
||||
|
||||
it('uses the current run messageId on CREATE (no claimed record)', async () => {
|
||||
mockClaimCodeFile.mockResolvedValue({
|
||||
file_id: 'mock-uuid-1234',
|
||||
user: 'user-123',
|
||||
});
|
||||
const persisted = snapshotCreateFileArgs();
|
||||
|
||||
const smallBuffer = Buffer.alloc(100);
|
||||
mockAxios.mockResolvedValue({ data: smallBuffer });
|
||||
|
||||
await processCodeOutput({
|
||||
...baseParams,
|
||||
messageId: 'turn-1-create-msg',
|
||||
});
|
||||
|
||||
expect(persisted[0].messageId).toBe('turn-1-create-msg');
|
||||
});
|
||||
|
||||
it('returns the CURRENT run messageId in the runtime response even on UPDATE (artifact attribution)', async () => {
|
||||
// The persisted DB record keeps the original messageId, but the
|
||||
// returned object surfaces the live messageId so the artifact lands
|
||||
// on the correct tool_call in this run's response.
|
||||
mockClaimCodeFile.mockResolvedValue({
|
||||
file_id: 'existing-id',
|
||||
filename: 'sentinel.txt',
|
||||
usage: 1,
|
||||
createdAt: '2024-01-01T00:00:00.000Z',
|
||||
messageId: 'turn-1-original-msg',
|
||||
});
|
||||
const persisted = snapshotCreateFileArgs();
|
||||
|
||||
const smallBuffer = Buffer.alloc(100);
|
||||
mockAxios.mockResolvedValue({ data: smallBuffer });
|
||||
|
||||
const result = await processCodeOutput({
|
||||
...baseParams,
|
||||
name: 'sentinel.txt',
|
||||
messageId: 'turn-2-current-run-msg',
|
||||
});
|
||||
|
||||
// DB preserves original
|
||||
expect(persisted[0].messageId).toBe('turn-1-original-msg');
|
||||
// Runtime return surfaces the live (current) messageId
|
||||
expect(result.messageId).toBe('turn-2-current-run-msg');
|
||||
});
|
||||
|
||||
it('preserves the original messageId on UPDATE for image files too', async () => {
|
||||
// Same contract as text files; the image branch builds its own file
|
||||
// record and would silently regress if the ternary diverged there.
|
||||
mockClaimCodeFile.mockResolvedValue({
|
||||
file_id: 'existing-img',
|
||||
filename: 'chart.png',
|
||||
usage: 1,
|
||||
createdAt: '2024-01-01T00:00:00.000Z',
|
||||
messageId: 'turn-1-image-msg',
|
||||
});
|
||||
const persisted = snapshotCreateFileArgs();
|
||||
|
||||
const imageBuffer = Buffer.alloc(500);
|
||||
mockAxios.mockResolvedValue({ data: imageBuffer });
|
||||
convertImage.mockResolvedValue({
|
||||
filepath: '/uploads/chart.webp',
|
||||
bytes: 400,
|
||||
});
|
||||
|
||||
await processCodeOutput({
|
||||
...baseParams,
|
||||
name: 'chart.png',
|
||||
messageId: 'turn-2-current-img-msg',
|
||||
});
|
||||
|
||||
expect(persisted[0].messageId).toBe('turn-1-image-msg');
|
||||
});
|
||||
});
|
||||
|
||||
describe('socket pool isolation', () => {
|
||||
it('should pass dedicated keepAlive:false agents to axios for processCodeOutput', async () => {
|
||||
const smallBuffer = Buffer.alloc(100);
|
||||
|
|
@ -523,4 +691,323 @@ describe('Code Process', () => {
|
|||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('readSandboxFile', () => {
|
||||
/**
|
||||
* `readSandboxFile` shells `cat <file_path>` through the sandbox
|
||||
* `/exec` endpoint. The `file_path` argument is model-controlled, so
|
||||
* the single-quote escaping is a security boundary — a regression
|
||||
* here would let a malicious filename break out of the `cat`
|
||||
* argument and inject arbitrary shell. Lock the contract in tests.
|
||||
*/
|
||||
|
||||
/** Pull the bash code that the helper would send to /exec, given
|
||||
* the file_path that the model supplied. */
|
||||
function execCodeFor(file_path) {
|
||||
mockAxios.mockResolvedValueOnce({ data: { stdout: '', stderr: '' } });
|
||||
return readSandboxFile({ file_path }).then(() => {
|
||||
const postData = mockAxios.mock.calls[0][0].data;
|
||||
return postData.code;
|
||||
});
|
||||
}
|
||||
|
||||
describe('shell quoting (security boundary)', () => {
|
||||
it('wraps a plain filename in single quotes', async () => {
|
||||
const code = await execCodeFor('/mnt/data/sentinel.txt');
|
||||
expect(code).toBe(`cat '/mnt/data/sentinel.txt'`);
|
||||
});
|
||||
|
||||
it("escapes a literal single-quote in the filename via the standard '\\'' sequence", async () => {
|
||||
// Adversarial filename: `quote'breakout.txt`. Naive
|
||||
// single-quoting would terminate the quoted string and
|
||||
// inject the trailing `breakout.txt'` as shell tokens.
|
||||
const code = await execCodeFor(`/mnt/data/quote'breakout.txt`);
|
||||
// Expected escape: end the string, escape a literal quote,
|
||||
// start a new string. POSIX-portable.
|
||||
expect(code).toBe(`cat '/mnt/data/quote'\\''breakout.txt'`);
|
||||
});
|
||||
|
||||
it('does not interpret command substitution syntax inside the quoted argument', async () => {
|
||||
// `$(rm -rf /)` would expand if the path were unquoted or
|
||||
// double-quoted. Inside POSIX single-quotes it stays literal.
|
||||
const code = await execCodeFor('/mnt/data/$(rm -rf /).txt');
|
||||
expect(code).toBe(`cat '/mnt/data/$(rm -rf /).txt'`);
|
||||
});
|
||||
|
||||
it('does not expand backtick command substitution inside the quoted argument', async () => {
|
||||
const code = await execCodeFor('/mnt/data/`whoami`.txt');
|
||||
expect(code).toBe(`cat '/mnt/data/\`whoami\`.txt'`);
|
||||
});
|
||||
|
||||
it('keeps newlines literal inside the quoted argument', async () => {
|
||||
const code = await execCodeFor('/mnt/data/line1\nline2.txt');
|
||||
expect(code).toBe(`cat '/mnt/data/line1\nline2.txt'`);
|
||||
});
|
||||
|
||||
it('keeps spaces and other shell metacharacters literal', async () => {
|
||||
const code = await execCodeFor('/mnt/data/file ; ls -la /etc/passwd');
|
||||
expect(code).toBe(`cat '/mnt/data/file ; ls -la /etc/passwd'`);
|
||||
});
|
||||
|
||||
it('handles multiple consecutive single-quotes', async () => {
|
||||
const code = await execCodeFor(`a''b`);
|
||||
// Each `'` becomes the 4-char escape sequence.
|
||||
expect(code).toBe(`cat 'a'\\'''\\''b'`);
|
||||
});
|
||||
});
|
||||
|
||||
describe('payload shape', () => {
|
||||
it('POSTs to /exec on the configured codeapi base URL with bash language', async () => {
|
||||
mockAxios.mockResolvedValueOnce({ data: { stdout: 'ok', stderr: '' } });
|
||||
|
||||
await readSandboxFile({ file_path: '/mnt/data/x.txt' });
|
||||
|
||||
const call = mockAxios.mock.calls[0][0];
|
||||
expect(call.method).toBe('post');
|
||||
expect(call.url).toBe('https://code-api.example.com/exec');
|
||||
expect(call.data.lang).toBe('bash');
|
||||
});
|
||||
|
||||
it('omits session_id and files when not provided', async () => {
|
||||
mockAxios.mockResolvedValueOnce({ data: { stdout: '', stderr: '' } });
|
||||
|
||||
await readSandboxFile({ file_path: '/mnt/data/x.txt' });
|
||||
|
||||
const data = mockAxios.mock.calls[0][0].data;
|
||||
expect(data).not.toHaveProperty('session_id');
|
||||
expect(data).not.toHaveProperty('files');
|
||||
});
|
||||
|
||||
it('forwards session_id when provided so the read lands in the seeded sandbox', async () => {
|
||||
mockAxios.mockResolvedValueOnce({ data: { stdout: '', stderr: '' } });
|
||||
|
||||
await readSandboxFile({
|
||||
file_path: '/mnt/data/x.txt',
|
||||
session_id: 'sess-XYZ',
|
||||
});
|
||||
|
||||
expect(mockAxios.mock.calls[0][0].data.session_id).toBe('sess-XYZ');
|
||||
});
|
||||
|
||||
it('forwards files (non-empty array) so prior-turn artifacts are mounted', async () => {
|
||||
mockAxios.mockResolvedValueOnce({ data: { stdout: '', stderr: '' } });
|
||||
|
||||
const files = [{ id: 'f1', name: 'sentinel.txt', session_id: 'sess-XYZ' }];
|
||||
await readSandboxFile({
|
||||
file_path: '/mnt/data/sentinel.txt',
|
||||
session_id: 'sess-XYZ',
|
||||
files,
|
||||
});
|
||||
|
||||
expect(mockAxios.mock.calls[0][0].data.files).toEqual(files);
|
||||
});
|
||||
|
||||
it('omits files when an empty array is provided (cleaner payload)', async () => {
|
||||
mockAxios.mockResolvedValueOnce({ data: { stdout: '', stderr: '' } });
|
||||
|
||||
await readSandboxFile({
|
||||
file_path: '/mnt/data/x.txt',
|
||||
session_id: 'sess-XYZ',
|
||||
files: [],
|
||||
});
|
||||
|
||||
expect(mockAxios.mock.calls[0][0].data).not.toHaveProperty('files');
|
||||
});
|
||||
|
||||
it('uses dedicated keepAlive:false agents (matches processCodeOutput pool isolation)', async () => {
|
||||
mockAxios.mockResolvedValueOnce({ data: { stdout: '', stderr: '' } });
|
||||
|
||||
await readSandboxFile({ file_path: '/mnt/data/x.txt' });
|
||||
|
||||
const call = mockAxios.mock.calls[0][0];
|
||||
expect(call.httpAgent).toBe(codeServerHttpAgent);
|
||||
expect(call.httpsAgent).toBe(codeServerHttpsAgent);
|
||||
});
|
||||
});
|
||||
|
||||
describe('response handling', () => {
|
||||
it('returns { content: stdout } on success', async () => {
|
||||
mockAxios.mockResolvedValueOnce({
|
||||
data: { stdout: 'sentinel-XYZ-1234\n', stderr: '' },
|
||||
});
|
||||
|
||||
const result = await readSandboxFile({ file_path: '/mnt/data/sentinel.txt' });
|
||||
|
||||
expect(result).toEqual({ content: 'sentinel-XYZ-1234\n' });
|
||||
});
|
||||
|
||||
it('returns null when getCodeBaseURL is not configured', async () => {
|
||||
const { getCodeBaseURL } = require('@librechat/agents');
|
||||
getCodeBaseURL.mockReturnValueOnce('');
|
||||
|
||||
const result = await readSandboxFile({ file_path: '/mnt/data/x.txt' });
|
||||
|
||||
expect(result).toBeNull();
|
||||
expect(mockAxios).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('returns null when stdout is missing entirely (no content to surface)', async () => {
|
||||
// stdout absent + no stderr = nothing to report; caller turns this
|
||||
// into a model-visible "Failed to read" message.
|
||||
mockAxios.mockResolvedValueOnce({ data: { stderr: '' } });
|
||||
|
||||
const result = await readSandboxFile({ file_path: '/mnt/data/x.txt' });
|
||||
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
|
||||
it('throws when the command writes to stderr with no stdout (exposes the error to the caller)', async () => {
|
||||
mockAxios.mockResolvedValueOnce({
|
||||
data: { stdout: '', stderr: 'cat: /mnt/data/missing.txt: No such file or directory\n' },
|
||||
});
|
||||
|
||||
await expect(readSandboxFile({ file_path: '/mnt/data/missing.txt' })).rejects.toThrow(
|
||||
'cat: /mnt/data/missing.txt: No such file or directory',
|
||||
);
|
||||
});
|
||||
|
||||
it('returns stdout even when stderr is also present (stdout wins on partial-success)', async () => {
|
||||
// Some `cat` builds emit warnings on stderr while still producing
|
||||
// stdout (e.g. unusual line endings). Surface the content.
|
||||
mockAxios.mockResolvedValueOnce({
|
||||
data: { stdout: 'partial', stderr: 'warning: ...' },
|
||||
});
|
||||
|
||||
const result = await readSandboxFile({ file_path: '/mnt/data/x.txt' });
|
||||
|
||||
expect(result).toEqual({ content: 'partial' });
|
||||
});
|
||||
|
||||
it('rethrows axios transport errors after logging via logAxiosError', async () => {
|
||||
const { logAxiosError } = require('@librechat/api');
|
||||
const transportError = Object.assign(new Error('connect ECONNREFUSED'), {
|
||||
code: 'ECONNREFUSED',
|
||||
});
|
||||
mockAxios.mockRejectedValueOnce(transportError);
|
||||
|
||||
await expect(readSandboxFile({ file_path: '/mnt/data/x.txt' })).rejects.toBe(
|
||||
transportError,
|
||||
);
|
||||
expect(logAxiosError).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
message: expect.stringContaining('/mnt/data/x.txt'),
|
||||
error: transportError,
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('timeout', () => {
|
||||
it('uses the same 15s timeout as processCodeOutput (consistent code-server SLA)', async () => {
|
||||
mockAxios.mockResolvedValueOnce({ data: { stdout: '', stderr: '' } });
|
||||
|
||||
await readSandboxFile({ file_path: '/mnt/data/x.txt' });
|
||||
|
||||
expect(mockAxios.mock.calls[0][0].timeout).toBe(15000);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('primeFiles reupload pushes FRESH sandbox ids (Pass-N review P2)', () => {
|
||||
/**
|
||||
* Regression: when a primed code file is missing/expired in the
|
||||
* sandbox (`getSessionInfo` returns null), `primeFiles` re-uploads
|
||||
* the file via `handleFileUpload` and persists the new
|
||||
* `fileIdentifier`. Before the fix, the in-memory `files[]` array
|
||||
* (now consumed by `buildInitialToolSessions` to seed
|
||||
* `Graph.sessions`) still received the STALE `(session_id, id)`
|
||||
* parsed from the original `fileIdentifier` at the top of the
|
||||
* loop. The DB record was correct but the seed referenced a
|
||||
* sandbox object that no longer existed — the first tool call
|
||||
* 404'd trying to mount it until the next turn re-read metadata.
|
||||
*
|
||||
* Fix: parse the FRESH `fileIdentifier` returned by upload and
|
||||
* push those ids into both the dedupe Map and the seed list.
|
||||
*/
|
||||
|
||||
const { getStrategyFunctions } = require('~/server/services/Files/strategies');
|
||||
const { updateFile, getFiles } = require('~/models');
|
||||
const { filterFilesByAgentAccess } = require('~/server/services/Files/permissions');
|
||||
|
||||
/**
|
||||
* Mock the full strategy pair. `primeFiles` calls
|
||||
* `getStrategyFunctions(file.source)` for the download stream and
|
||||
* `getStrategyFunctions(FileSources.execute_code)` for the code-env
|
||||
* upload — both go through the same factory in production.
|
||||
*/
|
||||
function setupReuploadMocks(newFileIdentifier) {
|
||||
const handleFileUpload = jest.fn().mockResolvedValue(newFileIdentifier);
|
||||
const getDownloadStream = jest.fn().mockResolvedValue('mock-stream');
|
||||
getStrategyFunctions.mockImplementation((source) => {
|
||||
if (source === 'execute_code') return { handleFileUpload };
|
||||
return { getDownloadStream };
|
||||
});
|
||||
updateFile.mockResolvedValue({});
|
||||
filterFilesByAgentAccess.mockImplementation(({ files }) => Promise.resolve(files));
|
||||
// getSessionInfo is mocked at module level via mockAxios; return null
|
||||
// to force the reupload path. Each `getSessionInfo` call hits axios.
|
||||
mockAxios.mockResolvedValue({ data: null });
|
||||
return { handleFileUpload, getDownloadStream };
|
||||
}
|
||||
|
||||
it('seed receives FRESH session_id + id parsed off the new fileIdentifier on reupload', async () => {
|
||||
const dbFile = {
|
||||
file_id: 'librechat-file-id',
|
||||
filename: 'sentinel.txt',
|
||||
filepath: '/uploads/sentinel.txt',
|
||||
source: 'local',
|
||||
context: 'execute_code',
|
||||
metadata: {
|
||||
/* Stale sandbox ref — this is what `getSessionInfo` will 404 on. */
|
||||
fileIdentifier: 'OLD_SESSION/OLD_ID',
|
||||
},
|
||||
};
|
||||
getFiles.mockResolvedValue([dbFile]);
|
||||
|
||||
setupReuploadMocks('NEW_SESSION/NEW_ID');
|
||||
|
||||
const result = await primeFiles({
|
||||
req: { user: { id: 'user-123', role: 'USER' } },
|
||||
tool_resources: {
|
||||
execute_code: { file_ids: ['librechat-file-id'], files: [] },
|
||||
},
|
||||
agentId: 'agent-id',
|
||||
});
|
||||
|
||||
// The seed list (consumed by buildInitialToolSessions) MUST carry
|
||||
// the post-reupload ids — not the stale pre-reupload ones.
|
||||
expect(result.files).toEqual([
|
||||
{ id: 'NEW_ID', session_id: 'NEW_SESSION', name: 'sentinel.txt' },
|
||||
]);
|
||||
});
|
||||
|
||||
it('persists the new fileIdentifier on the DB record (existing behavior, regression-locked)', async () => {
|
||||
const dbFile = {
|
||||
file_id: 'librechat-file-id',
|
||||
filename: 'sentinel.txt',
|
||||
filepath: '/uploads/sentinel.txt',
|
||||
source: 'local',
|
||||
context: 'execute_code',
|
||||
metadata: { fileIdentifier: 'OLD_SESSION/OLD_ID' },
|
||||
};
|
||||
getFiles.mockResolvedValue([dbFile]);
|
||||
|
||||
setupReuploadMocks('NEW_SESSION/NEW_ID');
|
||||
|
||||
await primeFiles({
|
||||
req: { user: { id: 'user-123', role: 'USER' } },
|
||||
tool_resources: {
|
||||
execute_code: { file_ids: ['librechat-file-id'], files: [] },
|
||||
},
|
||||
agentId: 'agent-id',
|
||||
});
|
||||
|
||||
expect(updateFile).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
file_id: 'librechat-file-id',
|
||||
metadata: expect.objectContaining({ fileIdentifier: 'NEW_SESSION/NEW_ID' }),
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -787,12 +787,27 @@ async function loadToolDefinitionsWrapper({ req, res, agent, streamId = null, to
|
|||
toolContextMap[Tools.web_search] = buildWebSearchContext();
|
||||
}
|
||||
|
||||
/**
|
||||
* `files` carry the upload session_ids; we surface them so client.js can
|
||||
* seed `Graph.sessions[EXECUTE_CODE]` before run start. Without that seed,
|
||||
* the agents-side `ToolNode.getCodeSessionContext` returns undefined on
|
||||
* call #1, `_injected_files` is never set on the tool call, and the
|
||||
* sandbox can't see the prior turn's generated artifacts on first read.
|
||||
*/
|
||||
let primedCodeFiles;
|
||||
if (hasExecuteCode && tool_resources) {
|
||||
try {
|
||||
const { toolContext } = await primeCodeFiles({ req, tool_resources, agentId: agent.id });
|
||||
const { toolContext, files } = await primeCodeFiles({
|
||||
req,
|
||||
tool_resources,
|
||||
agentId: agent.id,
|
||||
});
|
||||
if (toolContext) {
|
||||
toolContextMap[Tools.execute_code] = toolContext;
|
||||
}
|
||||
if (files?.length) {
|
||||
primedCodeFiles = files;
|
||||
}
|
||||
} catch (error) {
|
||||
logger.error('[loadToolDefinitionsWrapper] Error priming code files:', error);
|
||||
}
|
||||
|
|
@ -848,6 +863,7 @@ async function loadToolDefinitionsWrapper({ req, res, agent, streamId = null, to
|
|||
toolDefinitions,
|
||||
hasDeferredTools,
|
||||
actionsEnabled,
|
||||
primedCodeFiles,
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -946,7 +962,7 @@ async function loadAgentTools({
|
|||
});
|
||||
}
|
||||
|
||||
const { loadedTools, toolContextMap } = await loadTools({
|
||||
const { loadedTools, toolContextMap, primedCodeFiles } = await loadTools({
|
||||
agent,
|
||||
signal,
|
||||
userMCPAuthMap,
|
||||
|
|
@ -1035,6 +1051,7 @@ async function loadAgentTools({
|
|||
hasDeferredTools,
|
||||
actionsEnabled,
|
||||
tools: agentTools,
|
||||
primedCodeFiles,
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -1051,6 +1068,7 @@ async function loadAgentTools({
|
|||
hasDeferredTools,
|
||||
actionsEnabled,
|
||||
tools: agentTools,
|
||||
primedCodeFiles,
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -1174,6 +1192,7 @@ async function loadAgentTools({
|
|||
hasDeferredTools,
|
||||
actionsEnabled,
|
||||
tools: agentTools,
|
||||
primedCodeFiles,
|
||||
};
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,3 +1,24 @@
|
|||
/**
|
||||
* `undici` (transitive dep of `@librechat/agents` and others) references
|
||||
* `globalThis.File` from `node:buffer`. Node 20+ exposes it as a global;
|
||||
* Node 18 / certain WSL toolchains do not, which surfaces as a
|
||||
* `ReferenceError: File is not defined` at module-load time the first
|
||||
* time a test imports `@librechat/agents`. Mirror the polyfill in
|
||||
* `packages/api/jest.setup.cjs` so this Jest suite boots on the same
|
||||
* Node versions; production code never relies on this — only Jest does.
|
||||
*/
|
||||
if (typeof globalThis.File === 'undefined') {
|
||||
try {
|
||||
const { File } = require('node:buffer');
|
||||
if (File != null) {
|
||||
globalThis.File = File;
|
||||
}
|
||||
} catch {
|
||||
// Older Node versions without `node:buffer.File`. LibreChat doesn't
|
||||
// support those anyway; let the test fail loudly.
|
||||
}
|
||||
}
|
||||
|
||||
// See .env.test.example for an example of the '.env.test' file.
|
||||
require('dotenv').config({ path: './test/.env.test' });
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue