🧱 refactor: typed CodeEnvRef + kind discriminator + tenant-aware sandbox cache#12960
Merged
danny-avila merged 16 commits intodevfrom May 8, 2026
Merged
🧱 refactor: typed CodeEnvRef + kind discriminator + tenant-aware sandbox cache#12960danny-avila merged 16 commits intodevfrom
danny-avila merged 16 commits intodevfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors how code-execution sandbox file identity is represented and persisted by introducing a structured CodeEnvRef (instead of parsing/formatting a URL-shaped magic string in multiple places), while maintaining backward compatibility via dual-write/dual-read and providing a backfill migration script.
Changes:
- Added
CodeEnvRefplusparseCodeEnvIdentifier/formatCodeEnvIdentifier/resolveCodeEnvRefhelpers (with unit tests) to centralize legacy parsing and structured reads. - Updated schemas, types, and persistence paths to dual-write
codeEnvRefalongside legacycodeEnvIdentifier/metadata.fileIdentifier, and updated read paths to prefercodeEnvRef. - Added an idempotent migration script to backfill
codeEnvReffrom legacy identifiers.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/data-schemas/src/types/skill.ts | Adds codeEnvRef?: CodeEnvRef to ISkillFile and documents the legacy vs structured fields. |
| packages/data-schemas/src/types/file.ts | Adds metadata.codeEnvRef?: CodeEnvRef alongside legacy metadata.fileIdentifier. |
| packages/data-schemas/src/schema/skillFile.ts | Adds codeEnvRef subdocument to SkillFile schema. |
| packages/data-schemas/src/schema/file.ts | Adds metadata.codeEnvRef subdocument to File schema. |
| packages/data-schemas/src/methods/skill.ts | Extends updateSkillFileCodeEnvIds to optionally persist codeEnvRef in bulk updates. |
| packages/data-schemas/src/methods/skill.spec.ts | Adds tests covering dual-write and legacy-only persistence behavior for skill file code env IDs. |
| packages/data-schemas/src/methods/file.ts | Updates getCodeGeneratedFiles to match records that have either legacy or structured code env identifiers. |
| packages/data-provider/src/types/files.ts | Extends TFile.metadata to include optional codeEnvRef. |
| packages/data-provider/src/index.ts | Exports the new code-env ref helpers from the data-provider package entrypoint. |
| packages/data-provider/src/codeEnvRef.ts | Introduces CodeEnvRef and parse/format/resolve helpers for legacy identifiers and structured refs. |
| packages/data-provider/src/codeEnvRef.spec.ts | Adds unit tests for parsing/formatting and structured-vs-legacy resolution behavior. |
| packages/api/src/agents/skillFiles.ts | Replaces ad-hoc parsing with resolveCodeEnvRef and dual-writes codeEnvRef during skill priming. |
| packages/api/src/agents/skillFiles.spec.ts | Updates/extends tests for cache-hit behavior via codeEnvRef and fallback via legacy codeEnvIdentifier. |
| packages/api/src/agents/resources.ts | Treats metadata.codeEnvRef as equivalent to legacy metadata.fileIdentifier for execute_code resource categorization. |
| packages/api/src/agents/handlers.ts | Updates handler typings so updateSkillFileCodeEnvIds can accept optional codeEnvRef. |
| config/migrate-code-env-ref.js | Adds an idempotent backfill script to populate structured codeEnvRef from legacy identifiers. |
| api/server/services/Files/Code/process.spec.js | Updates tests to assert dual-write on metadata and dual-read behavior in primeFiles. |
| api/server/services/Files/Code/process.js | Uses resolveCodeEnvRef and dual-writes metadata.codeEnvRef while keeping legacy fileIdentifier for compatibility. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| */ | ||
| export function formatCodeEnvIdentifier(ref: CodeEnvRef): string { | ||
| const base = `${ref.storage_session_id}/${ref.file_id}`; | ||
| return ref.entity_id ? `${base}?entity_id=${ref.entity_id}` : base; |
Contributor
GitNexus: 🚀 deployedThe |
Contributor
GitNexus: 🚀 deployedThe |
Owner
Author
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 23bb3be934
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+1503
to
+1505
| $set: u.codeEnvRef | ||
| ? { codeEnvIdentifier: u.codeEnvIdentifier, codeEnvRef: u.codeEnvRef } | ||
| : { codeEnvIdentifier: u.codeEnvIdentifier }, |
There was a problem hiding this comment.
When an update omits codeEnvRef, this branch only writes codeEnvIdentifier and leaves any existing codeEnvRef untouched. Because resolveCodeEnvRef now prefers the structured field, readers can keep using an old (storage_session_id,file_id) even after the legacy identifier is refreshed, which can serve stale skill-file pointers during mixed-writer rollouts (or any caller still sending legacy-only updates).
Useful? React with 👍 / 👎.
Merged
3 tasks
danny-avila
added a commit
that referenced
this pull request
May 6, 2026
When `updateSkillFileCodeEnvIds` was called with `codeEnvIdentifier` but no `codeEnvRef`, the structured field on the row was left untouched. resolveCodeEnvRef prefers the structured field, so a mixed-writer rollout (or any caller still sending legacy-only updates) would shadow a freshly-refreshed legacy identifier with the old (storage_session_id, file_id) and serve stale skill-file pointers. Now the legacy-only branch $unsets `codeEnvRef` in the same op. Regression test added. Reported by Codex on #12960.
Owner
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Contributor
GitNexus: 🚀 deployedThe |
Contributor
GitNexus: 🚀 deployedThe |
Contributor
GitNexus: 🚀 deployedThe |
Contributor
GitNexus: 🚀 deployedThe |
danny-avila
added a commit
that referenced
this pull request
May 7, 2026
Companion to codeapi #1455 (option α). The upload endpoint now takes the same resource-identity triple `/exec` already does — without it, codeapi falls back to user bucketing for every upload and skill-cache invalidation (driven by the version bump on edit) silently never fires. Wire change in `Files/Code/crud.js`: - `uploadCodeEnvFile` and `batchUploadCodeEnvFiles` accept `kind`/`id`/ `version?` and forward them as multipart form fields. - `appendCodeEnvFileIdentity` validates the same shape codeapi's resolver enforces server-side: kind from the closed `{skill, agent, user}` set; `version` required for `skill` and forbidden otherwise. Bad callers fail fast on the client instead of round-tripping a 400. Call sites pass the identity from their own context: - `primeSkillFiles` / `primeInvokedSkills` — `kind: 'skill'`, `id: skill._id`, `version: skill.version` so codeapi keys under `<tenant>:skill:<id>:v:<version>`. Bumping version on edit naturally invalidates the prior cache entry. - `Files/process.js` (agent-upload path) — `kind: 'agent'` for files attached during agent setup, `kind: 'user'` for chat attachments. - `Code/process.js` (reupload path) — preserves `kind`/`id`/`version?` from the existing `codeEnvRef` so the re-bucket lands under the same sessionKey shape; otherwise a skill-cache-miss reupload would land in the user bucket and lose its cross-user shareability. Tests: - `crud.spec`: form fields appear / are validated / are absent on non-skill kinds. Asserts via `FormData.prototype.append` spy because `getBuffer()` chokes on the file-stream entry. - `process.spec`: reupload forwards kind/id/version from the existing ref (regression for the cross-user-shareability path). - `skillFiles.spec`: batchUpload receives `kind: 'skill'` + `id` + `version`. Deploys lockstep with codeapi #1455 — same boundary as the rest of #12960 / agents #148. No legacy alias; pre-release cutover.
Contributor
GitNexus: 🚀 deployedThe |
Contributor
GitNexus: 🚀 deployedThe |
danny-avila
changed the title
danny-avila
force-pushed
the
claude/happy-liskov-de7fcb
branch
from
7521912 to
211cfc6
Compare
Owner
Author
|
@codex review |
Contributor
GitNexus: 🚀 deployedThe |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 211cfc604c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| kind: codeKind, | ||
| id: codeId, | ||
| }); | ||
| fileInfoMetadata = { fileIdentifier }; |
There was a problem hiding this comment.
codeEnvRef metadata
uploadCodeEnvFile now returns a structured sandbox pointer, but this branch still stores it as metadata.fileIdentifier. In this commit, readers were switched to metadata.codeEnvRef (categorizeFileForToolResources, getCodeFilesByIds, and controller filtering), and the new schema only defines metadata.codeEnvRef, so execute-code uploads from processAgentFileUpload lose their sandbox reference and will not be mounted/reused in later code-execution turns.
Useful? React with 👍 / 👎.
Contributor
GitNexus: 🚀 deployedThe |
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 61bd50cb69
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
793
to
795
| id: overrideId ?? id, | ||
| session_id: overrideSessionId ?? session_id, | ||
| storage_session_id: overrideSessionId ?? session_id, | ||
| name: file.filename, |
There was a problem hiding this comment.
session_id when emitting primed code file refs
primeFiles() now pushes refs with storage_session_id only, but downstream seeding/injection still reads session_id (packages/api/src/agents/codeFilesSession.ts uses files[0].session_id for the representative session and file.session_id ?? codeSession.session_id when forwarding _injected_files). In runs that only have primed attachments (no skill seed), this makes seedCodeFilesIntoSessions drop the batch entirely, so the first execute_code call starts without injected files; in mixed-session runs it can also force files onto the wrong session fallback. Keep a compatible session_id field here (or update all session consumers in the same change).
Useful? React with 👍 / 👎.
Contributor
GitNexus: 🚀 deployedThe |
…box cache
Final cutover for the LibreChat ↔ codeapi sandbox file identity. Replaces
the magic string `${session_id}/${file_id}?entity_id=...` with a typed,
discriminated `CodeEnvRef`. Pre-release lockstep deploy with codeapi
#1455 and agents #148; no legacy aliases retained.
## Final shape
```ts
type CodeEnvRef =
| { kind: 'skill'; id: string; storage_session_id: string; file_id: string; version: number }
| { kind: 'agent'; id: string; storage_session_id: string; file_id: string }
| { kind: 'user'; id: string; storage_session_id: string; file_id: string };
```
`kind` drives codeapi's sessionKey: `<tenant>:<kind>:<id>[:v:<version>]`
for shared kinds, `<tenant>:user:<userId>` for user-private (auth context
provides `userId`). `version` is statically required for `kind: 'skill'`
and forbidden otherwise via discriminated union — constraint holds at
compile time on every consumer, not just codeapi's runtime validator.
`id` is sessionKey-meaningful for `'skill'` / `'agent'`; informational
only for `'user'` (codeapi resolves user identity from auth context).
## What changed
- `packages/data-provider/src/codeEnvRef.ts` — discriminated union +
`CODE_ENV_KINDS` const-tuple keeps the runtime list and TS union
locked together.
- Schemas: `metadata.codeEnvRef` and `SkillFile.codeEnvRef` enums
tightened to `['skill', 'agent', 'user']`.
- `primeSkillFiles` writes `kind: 'skill'`, `id: skill._id`,
`version: skill.version`. Cache-hit path reads `codeEnvRef`
directly. Bumping `skill.version` on edit naturally invalidates
the prior cache entry under the new sessionKey.
- `processCodeOutput` writes `kind: 'user'`, `id: req.user.id`. Output
bucket is always user-scoped, regardless of which skill the
execution invoked. New regression test pins the asymmetry.
- `primeFiles` reupload preserves `kind`/`id`/`version?` from the
existing ref so a skill-cache-miss reupload doesn't silently demote
to user bucket.
- `crud.js` upload functions (`uploadCodeEnvFile` /
`batchUploadCodeEnvFiles`) thread `kind`/`id`/`version?` to the
multipart form (codeapi #1455 option α). Without these on the wire,
codeapi falls back to user bucketing and skill-cache invalidation
never fires. Client-side validation mirrors codeapi's validator.
- `Files/process.js` — chat attachments use `kind: 'user'`; agent
setup files use `kind: 'agent'`.
- Drops `entity_id` everywhere (struct, schema sub-docs, write paths,
upload form fields). Drops `'system'` from the kind enum (no emitter
ever existed).
## Test plan
- [x] `cd packages/data-provider && npx jest src/codeEnvRef.spec` — 4 / 4
- [x] `cd packages/data-schemas && npx jest` — 1447 / 1447
- [x] `cd packages/api && npx jest src/agents` — 81 / 81 in skillFiles +
handlers + resources
- [x] `cd api && npx jest server/services/Files server/controllers/agents` —
436 / 436
- [x] `cd api && npx jest server/services/Files/Code` — 98 / 98 (incl.
new "outputs are user-scoped regardless of which skill the execution
invoked" regression and "reupload forwards kind/id/version from
existing ref")
- [x] `npx tsc --noEmit -p packages/data-{provider,schemas}/tsconfig.json
&& npx tsc --noEmit -p packages/api/tsconfig.json` — clean (only
pre-existing unrelated dev errors in storage/balance, untouched here)
## Deploy notes
- **24h cache-miss burst** on first deploy. Inputs (skill caches re-prime
under new sessionKey shape) and outputs (any pre-Phase C skill-output
cached files become unreadable). Bounded by codeapi's 24h TTL.
- **Lockstep with codeapi #1455 and agents #148.** Either repo can land
first since no aliases to drain, but the three deploys must overlap
within the same maintenance window.
- **`@librechat/agents` bump to `3.1.79-dev.0`** required after agents
#148 lands and is published.
## What this enables
Auth bridge work (JWT-based tenant/user identity between LC and codeapi)
— codeapi now derives sessionKey purely from `req.codeApiAuthContext.{
tenantId, userId}`, so the next chapter is replacing the header-asserted
user identity with a verified-claim path.
Codex review P1 (chatgpt-codex-connector). `Files/process.js` was
storing the upload result under `metadata.fileIdentifier` even though:
- `uploadCodeEnvFile` now returns `{ storage_session_id, file_id }`,
not the legacy magic string.
- The post-cutover schema (`File.metadata.codeEnvRef`) only declares
`codeEnvRef` — mongoose strict mode silently strips unknown keys.
- All readers (`primeFiles`, `getCodeFilesByIds`,
`categorizeFileForToolResources`, controller filtering) check
`metadata.codeEnvRef`.
Net effect of the bug: chat-attached and agent-setup execute_code files
would lose their sandbox reference on save, and primeFiles would skip
them on subsequent code-execution turns — the file blob would still be
available locally but never re-mounted in the sandbox.
Fix: construct the full `CodeEnvRef` (`{ kind, id, storage_session_id,
file_id }`) at the write site and persist under `metadata.codeEnvRef`.
`BaseClient`'s "is this a code-env file" presence check accepts the new
shape alongside the legacy `fileIdentifier` for back-compat with any
pre-cutover records still in the database. Mirrors the same change in
`processAttachments.spec.ts` (which re-implements the BaseClient logic
for testability).
New regression tests in `process.spec.js` cover three cases:
- chat attachments (`messageAttachment=true`) → `kind: 'user'`
- agent setup (`messageAttachment=false`) → `kind: 'agent'`
- legacy `fileIdentifier` key is NOT persisted (would be schema-stripped)
Contributor
GitNexus: 🚀 deployedThe |
Per the project convention that new backend code lives in TypeScript under `packages/api`, moves `appendCodeEnvFileIdentity` and `buildCodeEnvDownloadQuery` from `api/server/services/Files/Code/crud.js` into a new `packages/api/src/files/code/identity.ts` module. Both helpers are pure validators that mirror codeapi's `parseUploadSessionKeyInput` server-side rules (closed kind set, `version` required for `'skill'` and forbidden otherwise) — they deserve TS support and a dedicated spec rather than living as JSDoc-typed helpers in the legacy `/api` workspace. The new module: - Exports a `CodeEnvIdentity` interface using the `librechat-data-provider` `CodeEnvKind` discriminated union. - Adds 13 unit tests in `identity.spec.ts` covering the validation matrix (skill+version, agent, user, and every rejection path) plus URL encoding for the download query. - Re-exported from `packages/api/src/files/code/index.ts` alongside `classify`, `extract`, and `form`. Consumer updates: - `api/server/services/Files/Code/crud.js`: drops the local helpers and imports them from `@librechat/api`. Net -64 lines. - `api/server/services/Files/Code/process.js`: same. - Test mocks for `@librechat/api` in three spec files now stub the helpers' validation behavior locally rather than pulling them through `requireActual` (which would drag in provider-config init-time side effects). The package's `exports` field only surfaces the root barrel, so leaf imports aren't reachable from legacy `/api` test setup. No runtime behavior change. Identity validation rules and emitted form/query shapes are byte-for-byte identical pre/post.
Merged
5 tasks
Contributor
GitNexus: 🚀 deployedThe |
Companion to codeapi #1455 fix and agents 3.1.80-dev.1 — the wire shape for shared-kind files now requires `resource_id` distinct from the storage `id`. Without this LC change, codeapi's sessionKey re-derivation on every shared-kind /exec rejects with 403 session_key_mismatch: cached: legacy:skill:69dcf561...:v:59 (signed at upload, skill _id) derived: legacy:skill:ysPwEURuPk-...:v:59 (storage nanoid) Emit sites updated: - `primeInvokedSkills` cache-hit path: `resource_id: ref.id` (the persisted skill `_id` from `codeEnvRef.id`); `id: ref.file_id` unchanged (storage uuid). - `primeInvokedSkills` fresh-upload path: `resource_id: skill._id.toString()` on every primed file (the `allPrimedFiles` builder type now carries the field). - `processCodeOutput`'s `pushFile` (Code/process.js): `resource_id: ref.id` — for `kind: 'user'` this is informational (codeapi derives sessionKey from auth context) but emitted for shape uniformity with shared kinds. Bumps `@librechat/agents` to `^3.1.80-dev.1` (the version that ships the matching `CodeEnvFile.resource_id` field). ## Test plan - [x] `cd packages/api && npx jest src/agents` — 67 / 67 pass (skillFiles fixtures updated to assert `resource_id` on the emitted CodeSessionContext.files). - [x] `cd api && npx jest server/services/Files server/controllers/agents` — 445 / 445 pass (process.spec fixtures updated for the reupload + cache-hit emission). - [x] `npx tsc --noEmit -p packages/api/tsconfig.json` — clean.
Contributor
GitNexus: 🚀 deployedThe |
…ifact Codeapi was 400ing every /exec following a `handle_skill` tool call with `resource_id is invalid` (`type: 'undefined'`). Both code paths in `primeSkillFiles` (cache-hit + fresh-upload) returned files without `resource_id`/`kind`/`version`, and the artifact in `handlers.ts` forwarded the stripped shape into `tc.codeSessionContext.files` → `_injected_files`. `primeInvokedSkills` (the NL-detected loader) had already been fixed end-to-end; this commit aligns the tool-invoked path with the same contract: `resource_id` = `skill._id.toString()`, `kind: 'skill'`, `version` = the skill's monotonic counter. Tests added to `skillFiles.spec.ts` lock the contract on `primeSkillFiles` directly so future refactors can't silently drop the resource identity again.
…nd discriminator Pre-existing TS errors against the post-rename `CodeEnvFile` shape: the test file still used `session_id` on per-file objects (renamed to `storage_session_id` in agents Phase B/C) and was missing the `kind` discriminator the discriminated union requires. Both inputs and the matching `expect.toEqual(...)` mirrors updated together so the runtime equality check still holds. Lines 723-732 stay as-is — they sit behind `as unknown as ToolCallRequest` and TS already skipped them.
…ckage.json files
Contributor
GitNexus: 🚀 deployedThe |
…on and package-lock.json
Contributor
GitNexus: 🚀 deployedThe |
Contributor
GitNexus: 🚀 deployedThe |
…_injected_files
Diagnosing the user-upload "files=[] on first /exec" bug requires
seeing where in the LC chain a file ref disappears. Prior to this
patch the chain (primeCodeFiles → primedCodeFiles → initialSessions
→ CodeSessionContext → _injected_files) was opaque end-to-end:
- primeCodeFiles silently dropped files without `metadata.codeEnvRef`
- reuploadFile catches all errors and continues with no signal
- the handlers.ts handoff to codeapi never logged what it was sending
After this patch, a single grep on `[primeCodeFiles]` plus
`[code-env:inject]` shows the full per-file path:
[primeCodeFiles] in: file_ids=N resourceFiles=M
[primeCodeFiles] file=<id> path=skip reason=no-codeenvref filename=...
[primeCodeFiles] file=<id> path=cache-hit-by-session storage_session_id=...
[primeCodeFiles] file=<id> path=reupload reason=no-uploadtime ...
[primeCodeFiles] file=<id> path=reupload reason=stale ...
[primeCodeFiles] file=<id> path=reupload-success oldSession=... newSession=... newFileId=...
[primeCodeFiles] file=<id> path=reupload-failed session=...
[primeCodeFiles] file=<id> path=fresh-active storage_session_id=...
[primeCodeFiles] out: returned=N skippedNoRef=M reuploadFailures=K
[code-env:inject] tool=<name> files=N missingResourceId=K (debug)
[code-env:inject] M/N files missing resource_id ... (warn)
[code-env:inject] tool=<name> _injected_files=0 ... (warn)
The boundary log warns when LC sends zero injected files on a
code-execution tool call — that's the user's actual symptom showing
up at the LC side instead of having to correlate against codeapi's
`Request received { files: [] }`.
Tag chosen as `[code-env:inject]` rather than `[handoff:exec]` to
avoid collision with the app-level "handoff" semantic (subagent
handoff workflow).
Structural cleanup in primeFiles: replaced the `if (ref) { ... }`
nesting with an early `if (!ref) continue` so the per-path
instrumentation hooks land at top-level scope instead of indented
inside a conditional. Behavior unchanged; pushFile / reuploadFile
identical.
Spec fixtures (handlers.spec.ts, codeFilesSession.spec.ts) updated
to include `resource_id` on `CodeEnvFile` literals — required by
the post-3.1.80-dev.2 type now installed.
## Test plan
- [x] `cd packages/api && npx jest src/agents/handlers.spec.ts src/agents/codeFilesSession.spec.ts src/agents/skillFiles.spec.ts` — 69/69 pass
- [x] `cd api && npx jest server/services/Files/Code/process.spec.js` — 84/84 pass
- [x] `npx tsc --noEmit -p packages/api` — clean
- [x] `npx eslint` on all four touched files — clean
…ring length configuration
Contributor
GitNexus: 🚀 deployedThe |
…name
User-attached files for code execution were uploading to codeapi
under `file.originalname` (raw upload filename, may contain spaces /
special chars) while LC's DB record stored the sanitized form
(`sanitizeFilename(file.originalname)`, underscores). Codeapi
preserves whatever filename the upload sent, so the sandbox saw
`/mnt/data/<originalname>` while LC's `primeFiles` toolContext text
+ `_injected_files.name` referenced `file.filename` (sanitized).
Visible failure: agent gets system prompt saying
/mnt/data/librechat_code_api_-_active_customer_-_2025-11-05.xlsx
…tries that path, hits `FileNotFoundError`, then notices the
sandbox's actual `Available files` line says
/mnt/data/librechat code api - active customer - 2025-11-05.xlsx
…retries with spaces, succeeds. Wastes a tool call per upload and
leaks raw filenames into model context.
Fix: sanitize once and use the sanitized form in both the codeapi
upload AND the LC DB record. Sandbox path = LC toolContext text =
in-memory ref name. No drift.
Reupload path (`Code/process.js` line 867 `filename: file.filename`)
already uses the sanitized DB name, so it stays consistent with the
fresh-upload path after this change.
## Test plan
- [x] `cd api && npx jest server/services/Files/process` — 32/32 pass
- [x] `npx eslint` on the touched file — clean
Contributor
GitNexus: 🚀 deployedThe |
Owner
Author
|
@codex review |
…on and package-lock.json
|
Codex Review: Didn't find any major issues. Chef's kiss. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Contributor
GitNexus: 🚀 deployedThe |
danny-avila
added a commit
that referenced
this pull request
May 8, 2026
…geIds
In a branched conversation (regenerations producing the same code-output
filename), `getCodeGeneratedFiles` would silently exclude files whose
File-record `messageId` lived on a sibling branch. The user-visible
symptom: "the previous file isn't persisted" — the LLM tries
`load_workbook("output.xlsx")` on turn 2 and gets `FileNotFoundError`
because LC sent `_injected_files: []` to codeapi instead of priming
the prior turn's output.
`claimCodeFile` is keyed by `(filename, conversationId, context)` —
not by messageId. When sibling A first creates `output.csv`, the File
record persists with `messageId = A`. When sibling N (a regeneration
of A's parent) recreates `output.csv`, the claim finds A's record and
`processCodeOutput` deliberately preserves `messageId = A` to keep
file→original-creator provenance intact (correct behavior for the
linear case where the original creator is in-thread).
Turn N+1's `parentMessageId = N`. `getThreadData` walks back from N:
the thread is `[N, root]` — sibling A is NOT in it. The pre-fix query
filtered by `messageId IN [N, root]`, so the file was excluded.
`getCodeGeneratedFiles` already lives next to `getUserCodeFiles`,
which has always filtered by `file_id IN threadFileIds` (the file_ids
referenced by `messages.files[]` arrays during the thread walk). The
asymmetry — user-uploaded files anchored on the message's reference,
code-generated files anchored on the File's own creator — was the
bug. Anchoring both functions on `threadFileIds` reaches the right
files regardless of which sibling first generated them.
`File.messageId` stays informational ("who first generated this") for
provenance and `processCodeOutput`'s "preserve original messageId on
update" logic stays as-is — only the lookup key for thread-scoped
fetches changes.
- `packages/data-schemas/src/methods/file.ts`: signature + filter
change. JSDoc spells out the branched-conversation rationale.
- `packages/api/src/agents/initialize.ts`: pass `threadFileIds` instead
of `threadMessageIds`. The local `threadMessageIds` declaration is
removed since the only consumer is gone.
- `packages/data-schemas/src/methods/file.spec.ts`: 5 new cases:
- basic happy-path (file referenced by current thread)
- **the regression**: file's creator messageId is on a sibling
branch but file_id is in threadFileIds → finds it
- empty/missing threadFileIds returns []
- cross-conversation isolation
- non-execute_code context filter still applies (a chat attachment
won't be returned even if its file_id is in threadFileIds —
that's `getUserCodeFiles`'s job)
Applies cleanly on top of dev. When LC #12960 (the typed CodeEnvRef
cutover) lands, the only conflict is the legacy `metadata.fileIdentifier`
metadata key flipping to `metadata.codeEnvRef` — same line, trivial
resolve.
- [x] `cd packages/data-schemas && npx jest src/methods/file.spec` —
42/42 pass (including the 5 new regression cases)
- [x] `cd packages/api && npx jest src/agents` — 722/722 pass
(modulo 2 pre-existing summarization e2e failures unrelated)
- [x] `cd api && npx jest server/services/Files server/controllers/agents` —
432/432 pass
- [x] `npx tsc --noEmit -p packages/api/tsconfig.json` — clean
- [ ] Manual: branched conversation reproducer — generate a file in
turn 1, regenerate the parent (sibling), then in turn N+1 ask the
agent to read the file. Pre-fix: `FileNotFoundError`. Post-fix:
the file is primed and load_workbook succeeds.
danny-avila
added a commit
that referenced
this pull request
May 8, 2026
…anched conversations (#13004) * 🐛 fix: anchor getCodeGeneratedFiles on threadFileIds, not threadMessageIds In a branched conversation (regenerations producing the same code-output filename), `getCodeGeneratedFiles` would silently exclude files whose File-record `messageId` lived on a sibling branch. The user-visible symptom: "the previous file isn't persisted" — the LLM tries `load_workbook("output.xlsx")` on turn 2 and gets `FileNotFoundError` because LC sent `_injected_files: []` to codeapi instead of priming the prior turn's output. `claimCodeFile` is keyed by `(filename, conversationId, context)` — not by messageId. When sibling A first creates `output.csv`, the File record persists with `messageId = A`. When sibling N (a regeneration of A's parent) recreates `output.csv`, the claim finds A's record and `processCodeOutput` deliberately preserves `messageId = A` to keep file→original-creator provenance intact (correct behavior for the linear case where the original creator is in-thread). Turn N+1's `parentMessageId = N`. `getThreadData` walks back from N: the thread is `[N, root]` — sibling A is NOT in it. The pre-fix query filtered by `messageId IN [N, root]`, so the file was excluded. `getCodeGeneratedFiles` already lives next to `getUserCodeFiles`, which has always filtered by `file_id IN threadFileIds` (the file_ids referenced by `messages.files[]` arrays during the thread walk). The asymmetry — user-uploaded files anchored on the message's reference, code-generated files anchored on the File's own creator — was the bug. Anchoring both functions on `threadFileIds` reaches the right files regardless of which sibling first generated them. `File.messageId` stays informational ("who first generated this") for provenance and `processCodeOutput`'s "preserve original messageId on update" logic stays as-is — only the lookup key for thread-scoped fetches changes. - `packages/data-schemas/src/methods/file.ts`: signature + filter change. JSDoc spells out the branched-conversation rationale. - `packages/api/src/agents/initialize.ts`: pass `threadFileIds` instead of `threadMessageIds`. The local `threadMessageIds` declaration is removed since the only consumer is gone. - `packages/data-schemas/src/methods/file.spec.ts`: 5 new cases: - basic happy-path (file referenced by current thread) - **the regression**: file's creator messageId is on a sibling branch but file_id is in threadFileIds → finds it - empty/missing threadFileIds returns [] - cross-conversation isolation - non-execute_code context filter still applies (a chat attachment won't be returned even if its file_id is in threadFileIds — that's `getUserCodeFiles`'s job) Applies cleanly on top of dev. When LC #12960 (the typed CodeEnvRef cutover) lands, the only conflict is the legacy `metadata.fileIdentifier` metadata key flipping to `metadata.codeEnvRef` — same line, trivial resolve. - [x] `cd packages/data-schemas && npx jest src/methods/file.spec` — 42/42 pass (including the 5 new regression cases) - [x] `cd packages/api && npx jest src/agents` — 722/722 pass (modulo 2 pre-existing summarization e2e failures unrelated) - [x] `cd api && npx jest server/services/Files server/controllers/agents` — 432/432 pass - [x] `npx tsc --noEmit -p packages/api/tsconfig.json` — clean - [ ] Manual: branched conversation reproducer — generate a file in turn 1, regenerate the parent (sibling), then in turn N+1 ask the agent to read the file. Pre-fix: `FileNotFoundError`. Post-fix: the file is primed and load_workbook succeeds. * 🧪 test: lock initialize.ts → getCodeGeneratedFiles call shape Integration-level regression test asserting initializeAgent passes `threadFileIds` (not `threadMessageIds`) to getCodeGeneratedFiles in branched-conversation scenarios. Locks in the API shape from the previous commit, sitting one layer above the data-schemas unit test — so a future refactor to the priming chain can't silently revert to the messageId-based filter without surfacing a test failure here. Two cases: - The full call shape: agent.tools=['execute_code'], resendFiles=true, threadData mock returns distinct messageIds and fileIds. Asserts the call uses fileIds, and that getUserCodeFiles uses the same array (the symmetric design that closes the sibling-branch hole). - Empty threadFileIds: getCodeGeneratedFiles is still called with [] (its own internal early-return handles the empty case); getUserCodeFiles is gated at the call site and stays unscheduled.
danny-avila
added a commit
that referenced
this pull request
May 8, 2026
…andbox cache (#12960) * 🧱 refactor: typed CodeEnvRef + kind discriminator + tenant-aware sandbox cache Final cutover for the LibreChat ↔ codeapi sandbox file identity. Replaces the magic string `${session_id}/${file_id}?entity_id=...` with a typed, discriminated `CodeEnvRef`. Pre-release lockstep deploy with codeapi #1455 and agents #148; no legacy aliases retained. ## Final shape ```ts type CodeEnvRef = | { kind: 'skill'; id: string; storage_session_id: string; file_id: string; version: number } | { kind: 'agent'; id: string; storage_session_id: string; file_id: string } | { kind: 'user'; id: string; storage_session_id: string; file_id: string }; ``` `kind` drives codeapi's sessionKey: `<tenant>:<kind>:<id>[:v:<version>]` for shared kinds, `<tenant>:user:<userId>` for user-private (auth context provides `userId`). `version` is statically required for `kind: 'skill'` and forbidden otherwise via discriminated union — constraint holds at compile time on every consumer, not just codeapi's runtime validator. `id` is sessionKey-meaningful for `'skill'` / `'agent'`; informational only for `'user'` (codeapi resolves user identity from auth context). ## What changed - `packages/data-provider/src/codeEnvRef.ts` — discriminated union + `CODE_ENV_KINDS` const-tuple keeps the runtime list and TS union locked together. - Schemas: `metadata.codeEnvRef` and `SkillFile.codeEnvRef` enums tightened to `['skill', 'agent', 'user']`. - `primeSkillFiles` writes `kind: 'skill'`, `id: skill._id`, `version: skill.version`. Cache-hit path reads `codeEnvRef` directly. Bumping `skill.version` on edit naturally invalidates the prior cache entry under the new sessionKey. - `processCodeOutput` writes `kind: 'user'`, `id: req.user.id`. Output bucket is always user-scoped, regardless of which skill the execution invoked. New regression test pins the asymmetry. - `primeFiles` reupload preserves `kind`/`id`/`version?` from the existing ref so a skill-cache-miss reupload doesn't silently demote to user bucket. - `crud.js` upload functions (`uploadCodeEnvFile` / `batchUploadCodeEnvFiles`) thread `kind`/`id`/`version?` to the multipart form (codeapi #1455 option α). Without these on the wire, codeapi falls back to user bucketing and skill-cache invalidation never fires. Client-side validation mirrors codeapi's validator. - `Files/process.js` — chat attachments use `kind: 'user'`; agent setup files use `kind: 'agent'`. - Drops `entity_id` everywhere (struct, schema sub-docs, write paths, upload form fields). Drops `'system'` from the kind enum (no emitter ever existed). ## Test plan - [x] `cd packages/data-provider && npx jest src/codeEnvRef.spec` — 4 / 4 - [x] `cd packages/data-schemas && npx jest` — 1447 / 1447 - [x] `cd packages/api && npx jest src/agents` — 81 / 81 in skillFiles + handlers + resources - [x] `cd api && npx jest server/services/Files server/controllers/agents` — 436 / 436 - [x] `cd api && npx jest server/services/Files/Code` — 98 / 98 (incl. new "outputs are user-scoped regardless of which skill the execution invoked" regression and "reupload forwards kind/id/version from existing ref") - [x] `npx tsc --noEmit -p packages/data-{provider,schemas}/tsconfig.json && npx tsc --noEmit -p packages/api/tsconfig.json` — clean (only pre-existing unrelated dev errors in storage/balance, untouched here) ## Deploy notes - **24h cache-miss burst** on first deploy. Inputs (skill caches re-prime under new sessionKey shape) and outputs (any pre-Phase C skill-output cached files become unreadable). Bounded by codeapi's 24h TTL. - **Lockstep with codeapi #1455 and agents #148.** Either repo can land first since no aliases to drain, but the three deploys must overlap within the same maintenance window. - **`@librechat/agents` bump to `3.1.79-dev.0`** required after agents #148 lands and is published. ## What this enables Auth bridge work (JWT-based tenant/user identity between LC and codeapi) — codeapi now derives sessionKey purely from `req.codeApiAuthContext.{ tenantId, userId}`, so the next chapter is replacing the header-asserted user identity with a verified-claim path. * 🩹 fix: persist execute_code uploads under codeEnvRef metadata key Codex review P1 (chatgpt-codex-connector). `Files/process.js` was storing the upload result under `metadata.fileIdentifier` even though: - `uploadCodeEnvFile` now returns `{ storage_session_id, file_id }`, not the legacy magic string. - The post-cutover schema (`File.metadata.codeEnvRef`) only declares `codeEnvRef` — mongoose strict mode silently strips unknown keys. - All readers (`primeFiles`, `getCodeFilesByIds`, `categorizeFileForToolResources`, controller filtering) check `metadata.codeEnvRef`. Net effect of the bug: chat-attached and agent-setup execute_code files would lose their sandbox reference on save, and primeFiles would skip them on subsequent code-execution turns — the file blob would still be available locally but never re-mounted in the sandbox. Fix: construct the full `CodeEnvRef` (`{ kind, id, storage_session_id, file_id }`) at the write site and persist under `metadata.codeEnvRef`. `BaseClient`'s "is this a code-env file" presence check accepts the new shape alongside the legacy `fileIdentifier` for back-compat with any pre-cutover records still in the database. Mirrors the same change in `processAttachments.spec.ts` (which re-implements the BaseClient logic for testability). New regression tests in `process.spec.js` cover three cases: - chat attachments (`messageAttachment=true`) → `kind: 'user'` - agent setup (`messageAttachment=false`) → `kind: 'agent'` - legacy `fileIdentifier` key is NOT persisted (would be schema-stripped) * 🩹 fix: read storage_session_id on primed file refs (Codex P1) Codex review (chatgpt-codex-connector). After Phase B's per-file `session_id` → `storage_session_id` rename, `primeFiles` emits the new field — but `seedCodeFilesIntoSessions` was still reading `files[0].session_id` for the representative session and `f.session_id` for the dedupe key. In runs with only primed attachments (no skill seed), `representativeSessionId` was `undefined`, the function returned the unchanged map, and `seedCodeFilesIntoSessions` silently dropped the entire batch. The first `execute_code` call then started without `_injected_files` and the agent couldn't see prior-turn artifacts. Fix: - `codeFilesSession.ts`: read `f.storage_session_id` for both the dedupe key and the representative session id. JSDoc updated to match the new field name. - `callbacks.js`: the two output-file persistence paths read `file.session_id` to pass to `processCodeOutput` — switch to `file.storage_session_id`. The original comment explicitly says this should be the STORAGE session, which is exactly the field Phase B renamed. - `codeFilesSession.spec.ts`: fixture builder uses `storage_session_id` and `kind: 'user'` to match the post-cutover `CodeEnvFile` shape. Lockstep coordination: this matches the post-bump shape of `@librechat/agents` 3.1.79+. CI tsc errors against the currently-pinned 3.1.78 are expected and resolve when the dep bumps in this PR before merge. * 📦 chore: Bump `@librechat/agents` to version 3.1.80-dev.0 in package-lock and package.json files * 🪪 fix: thread kind/id/version through codeapi /download URLs (Phase C α) Symmetric fix for the upload-side wire change in 537725a. Codeapi's `sessionAuth` middleware now requires `kind`/`id`/`version?` on every download/freshness URL — without them it 400s with "kind must be one of: skill, agent, user" before serving the file. Three sites construct codeapi-side URLs that go through `sessionAuth`: - `processCodeOutput` (`Files/Code/process.js`): `/download/<sess>/<id>` for freshly-generated sandbox outputs. Always `kind: 'user'` + `id: req.user.id` — code-output files are always user-private, regardless of which skill the run invoked. - `getSessionInfo` (`Files/Code/process.js`): `/sessions/<sess>/objects/<id>` for the 23h freshness check. Pulls kind/id/version straight off the `codeEnvRef` already in scope — skill files stay skill-bucketed, user files stay user-bucketed. - `/code/download/:session_id/:fileId` LC route (`routes/files/files.js`): proxies to codeapi for manual downloads. Code-output files only on this route, so `kind: 'user'` + `id: req.user.id`. The `getCodeOutputDownloadStream` helper in `crud.js` now takes an `identity` param, validated by a `buildCodeEnvDownloadQuery` helper that mirrors `appendCodeEnvFileIdentity`'s shape rules: kind required from the closed `{skill, agent, user}` set, version required for 'skill' and forbidden otherwise. Bad callers fail fast on the client instead of round-tripping a 400. Also cleans up two log-noise sources reported alongside the 400: - `logAxiosError` in `packages/api/src/utils/axios.ts` was dumping `error.response.data` raw. With `responseType: 'arraybuffer'` that's a `Buffer` (~4 chars per byte after JSON-serialization); with `responseType: 'stream'` it's a `Readable` whose internal state serializes the entire ring buffer + socket. New `renderResponseData` decodes small buffers as UTF-8 (truncated past 2KB) and stubs streams as `'[stream]'`. Diagnostics stay useful, log lines stop being megabytes. - `/code/download` route's catch was bare `logger.error('...', error)`, bypassing the redactor. Switched to `logAxiosError` so it benefits from the same buffer/stream handling. Tests updated to match the new contract: - crud.spec: `getCodeOutputDownloadStream` fixtures pass `userIdentity`; new cases cover skill identity (with version), bad kind rejection, skill-without-version rejection. - process.spec: `getSessionInfo` test passes a full `codeEnvRef` object. * ♻️ refactor: extract codeEnv identity helpers into packages/api Per the project convention that new backend code lives in TypeScript under `packages/api`, moves `appendCodeEnvFileIdentity` and `buildCodeEnvDownloadQuery` from `api/server/services/Files/Code/crud.js` into a new `packages/api/src/files/code/identity.ts` module. Both helpers are pure validators that mirror codeapi's `parseUploadSessionKeyInput` server-side rules (closed kind set, `version` required for `'skill'` and forbidden otherwise) — they deserve TS support and a dedicated spec rather than living as JSDoc-typed helpers in the legacy `/api` workspace. The new module: - Exports a `CodeEnvIdentity` interface using the `librechat-data-provider` `CodeEnvKind` discriminated union. - Adds 13 unit tests in `identity.spec.ts` covering the validation matrix (skill+version, agent, user, and every rejection path) plus URL encoding for the download query. - Re-exported from `packages/api/src/files/code/index.ts` alongside `classify`, `extract`, and `form`. Consumer updates: - `api/server/services/Files/Code/crud.js`: drops the local helpers and imports them from `@librechat/api`. Net -64 lines. - `api/server/services/Files/Code/process.js`: same. - Test mocks for `@librechat/api` in three spec files now stub the helpers' validation behavior locally rather than pulling them through `requireActual` (which would drag in provider-config init-time side effects). The package's `exports` field only surfaces the root barrel, so leaf imports aren't reachable from legacy `/api` test setup. No runtime behavior change. Identity validation rules and emitted form/query shapes are byte-for-byte identical pre/post. * 🪪 fix: emit resource_id alongside id on _injected_files (skill 403 fix) Companion to codeapi #1455 fix and agents 3.1.80-dev.1 — the wire shape for shared-kind files now requires `resource_id` distinct from the storage `id`. Without this LC change, codeapi's sessionKey re-derivation on every shared-kind /exec rejects with 403 session_key_mismatch: cached: legacy:skill:69dcf561...:v:59 (signed at upload, skill _id) derived: legacy:skill:ysPwEURuPk-...:v:59 (storage nanoid) Emit sites updated: - `primeInvokedSkills` cache-hit path: `resource_id: ref.id` (the persisted skill `_id` from `codeEnvRef.id`); `id: ref.file_id` unchanged (storage uuid). - `primeInvokedSkills` fresh-upload path: `resource_id: skill._id.toString()` on every primed file (the `allPrimedFiles` builder type now carries the field). - `processCodeOutput`'s `pushFile` (Code/process.js): `resource_id: ref.id` — for `kind: 'user'` this is informational (codeapi derives sessionKey from auth context) but emitted for shape uniformity with shared kinds. Bumps `@librechat/agents` to `^3.1.80-dev.1` (the version that ships the matching `CodeEnvFile.resource_id` field). ## Test plan - [x] `cd packages/api && npx jest src/agents` — 67 / 67 pass (skillFiles fixtures updated to assert `resource_id` on the emitted CodeSessionContext.files). - [x] `cd api && npx jest server/services/Files server/controllers/agents` — 445 / 445 pass (process.spec fixtures updated for the reupload + cache-hit emission). - [x] `npx tsc --noEmit -p packages/api/tsconfig.json` — clean. * fix(skill-tool-call): carry resource_id through primeSkillFiles → artifact Codeapi was 400ing every /exec following a `handle_skill` tool call with `resource_id is invalid` (`type: 'undefined'`). Both code paths in `primeSkillFiles` (cache-hit + fresh-upload) returned files without `resource_id`/`kind`/`version`, and the artifact in `handlers.ts` forwarded the stripped shape into `tc.codeSessionContext.files` → `_injected_files`. `primeInvokedSkills` (the NL-detected loader) had already been fixed end-to-end; this commit aligns the tool-invoked path with the same contract: `resource_id` = `skill._id.toString()`, `kind: 'skill'`, `version` = the skill's monotonic counter. Tests added to `skillFiles.spec.ts` lock the contract on `primeSkillFiles` directly so future refactors can't silently drop the resource identity again. * fix(handlers.spec): align session_id → storage_session_id rename + kind discriminator Pre-existing TS errors against the post-rename `CodeEnvFile` shape: the test file still used `session_id` on per-file objects (renamed to `storage_session_id` in agents Phase B/C) and was missing the `kind` discriminator the discriminated union requires. Both inputs and the matching `expect.toEqual(...)` mirrors updated together so the runtime equality check still holds. Lines 723-732 stay as-is — they sit behind `as unknown as ToolCallRequest` and TS already skipped them. * chore: fix `@librechat/agents`, correct version to 3.1.80-dev.0 in package.json files * chore: bump `@librechat/agents` to version 3.1.80-dev.1 in package.json and package-lock.json * chore: bump `@librechat/agents` to version 3.1.80-dev.2 * feat(observability): trace file priming chain from primeCodeFiles to _injected_files Diagnosing the user-upload "files=[] on first /exec" bug requires seeing where in the LC chain a file ref disappears. Prior to this patch the chain (primeCodeFiles → primedCodeFiles → initialSessions → CodeSessionContext → _injected_files) was opaque end-to-end: - primeCodeFiles silently dropped files without `metadata.codeEnvRef` - reuploadFile catches all errors and continues with no signal - the handlers.ts handoff to codeapi never logged what it was sending After this patch, a single grep on `[primeCodeFiles]` plus `[code-env:inject]` shows the full per-file path: [primeCodeFiles] in: file_ids=N resourceFiles=M [primeCodeFiles] file=<id> path=skip reason=no-codeenvref filename=... [primeCodeFiles] file=<id> path=cache-hit-by-session storage_session_id=... [primeCodeFiles] file=<id> path=reupload reason=no-uploadtime ... [primeCodeFiles] file=<id> path=reupload reason=stale ... [primeCodeFiles] file=<id> path=reupload-success oldSession=... newSession=... newFileId=... [primeCodeFiles] file=<id> path=reupload-failed session=... [primeCodeFiles] file=<id> path=fresh-active storage_session_id=... [primeCodeFiles] out: returned=N skippedNoRef=M reuploadFailures=K [code-env:inject] tool=<name> files=N missingResourceId=K (debug) [code-env:inject] M/N files missing resource_id ... (warn) [code-env:inject] tool=<name> _injected_files=0 ... (warn) The boundary log warns when LC sends zero injected files on a code-execution tool call — that's the user's actual symptom showing up at the LC side instead of having to correlate against codeapi's `Request received { files: [] }`. Tag chosen as `[code-env:inject]` rather than `[handoff:exec]` to avoid collision with the app-level "handoff" semantic (subagent handoff workflow). Structural cleanup in primeFiles: replaced the `if (ref) { ... }` nesting with an early `if (!ref) continue` so the per-path instrumentation hooks land at top-level scope instead of indented inside a conditional. Behavior unchanged; pushFile / reuploadFile identical. Spec fixtures (handlers.spec.ts, codeFilesSession.spec.ts) updated to include `resource_id` on `CodeEnvFile` literals — required by the post-3.1.80-dev.2 type now installed. ## Test plan - [x] `cd packages/api && npx jest src/agents/handlers.spec.ts src/agents/codeFilesSession.spec.ts src/agents/skillFiles.spec.ts` — 69/69 pass - [x] `cd api && npx jest server/services/Files/Code/process.spec.js` — 84/84 pass - [x] `npx tsc --noEmit -p packages/api` — clean - [x] `npx eslint` on all four touched files — clean * chore: add CONSOLE_JSON_STRING_LENGTH to .env.example for JSON log string length configuration * fix(files): align codeapi upload filename with LC's sanitized DB filename User-attached files for code execution were uploading to codeapi under `file.originalname` (raw upload filename, may contain spaces / special chars) while LC's DB record stored the sanitized form (`sanitizeFilename(file.originalname)`, underscores). Codeapi preserves whatever filename the upload sent, so the sandbox saw `/mnt/data/<originalname>` while LC's `primeFiles` toolContext text + `_injected_files.name` referenced `file.filename` (sanitized). Visible failure: agent gets system prompt saying /mnt/data/librechat_code_api_-_active_customer_-_2025-11-05.xlsx …tries that path, hits `FileNotFoundError`, then notices the sandbox's actual `Available files` line says /mnt/data/librechat code api - active customer - 2025-11-05.xlsx …retries with spaces, succeeds. Wastes a tool call per upload and leaks raw filenames into model context. Fix: sanitize once and use the sanitized form in both the codeapi upload AND the LC DB record. Sandbox path = LC toolContext text = in-memory ref name. No drift. Reupload path (`Code/process.js` line 867 `filename: file.filename`) already uses the sanitized DB name, so it stays consistent with the fresh-upload path after this change. ## Test plan - [x] `cd api && npx jest server/services/Files/process` — 32/32 pass - [x] `npx eslint` on the touched file — clean * chore: bump `@librechat/agents` to version 3.1.80-dev.3 in package.json and package-lock.json
danny-avila
added a commit
that referenced
this pull request
May 8, 2026
…anched conversations (#13004) * 🐛 fix: anchor getCodeGeneratedFiles on threadFileIds, not threadMessageIds In a branched conversation (regenerations producing the same code-output filename), `getCodeGeneratedFiles` would silently exclude files whose File-record `messageId` lived on a sibling branch. The user-visible symptom: "the previous file isn't persisted" — the LLM tries `load_workbook("output.xlsx")` on turn 2 and gets `FileNotFoundError` because LC sent `_injected_files: []` to codeapi instead of priming the prior turn's output. `claimCodeFile` is keyed by `(filename, conversationId, context)` — not by messageId. When sibling A first creates `output.csv`, the File record persists with `messageId = A`. When sibling N (a regeneration of A's parent) recreates `output.csv`, the claim finds A's record and `processCodeOutput` deliberately preserves `messageId = A` to keep file→original-creator provenance intact (correct behavior for the linear case where the original creator is in-thread). Turn N+1's `parentMessageId = N`. `getThreadData` walks back from N: the thread is `[N, root]` — sibling A is NOT in it. The pre-fix query filtered by `messageId IN [N, root]`, so the file was excluded. `getCodeGeneratedFiles` already lives next to `getUserCodeFiles`, which has always filtered by `file_id IN threadFileIds` (the file_ids referenced by `messages.files[]` arrays during the thread walk). The asymmetry — user-uploaded files anchored on the message's reference, code-generated files anchored on the File's own creator — was the bug. Anchoring both functions on `threadFileIds` reaches the right files regardless of which sibling first generated them. `File.messageId` stays informational ("who first generated this") for provenance and `processCodeOutput`'s "preserve original messageId on update" logic stays as-is — only the lookup key for thread-scoped fetches changes. - `packages/data-schemas/src/methods/file.ts`: signature + filter change. JSDoc spells out the branched-conversation rationale. - `packages/api/src/agents/initialize.ts`: pass `threadFileIds` instead of `threadMessageIds`. The local `threadMessageIds` declaration is removed since the only consumer is gone. - `packages/data-schemas/src/methods/file.spec.ts`: 5 new cases: - basic happy-path (file referenced by current thread) - **the regression**: file's creator messageId is on a sibling branch but file_id is in threadFileIds → finds it - empty/missing threadFileIds returns [] - cross-conversation isolation - non-execute_code context filter still applies (a chat attachment won't be returned even if its file_id is in threadFileIds — that's `getUserCodeFiles`'s job) Applies cleanly on top of dev. When LC #12960 (the typed CodeEnvRef cutover) lands, the only conflict is the legacy `metadata.fileIdentifier` metadata key flipping to `metadata.codeEnvRef` — same line, trivial resolve. - [x] `cd packages/data-schemas && npx jest src/methods/file.spec` — 42/42 pass (including the 5 new regression cases) - [x] `cd packages/api && npx jest src/agents` — 722/722 pass (modulo 2 pre-existing summarization e2e failures unrelated) - [x] `cd api && npx jest server/services/Files server/controllers/agents` — 432/432 pass - [x] `npx tsc --noEmit -p packages/api/tsconfig.json` — clean - [ ] Manual: branched conversation reproducer — generate a file in turn 1, regenerate the parent (sibling), then in turn N+1 ask the agent to read the file. Pre-fix: `FileNotFoundError`. Post-fix: the file is primed and load_workbook succeeds. * 🧪 test: lock initialize.ts → getCodeGeneratedFiles call shape Integration-level regression test asserting initializeAgent passes `threadFileIds` (not `threadMessageIds`) to getCodeGeneratedFiles in branched-conversation scenarios. Locks in the API shape from the previous commit, sitting one layer above the data-schemas unit test — so a future refactor to the priming chain can't silently revert to the messageId-based filter without surfacing a test failure here. Two cases: - The full call shape: agent.tools=['execute_code'], resendFiles=true, threadData mock returns distinct messageIds and fileIds. Asserts the call uses fileIds, and that getUserCodeFiles uses the same array (the symmetric design that closes the sibling-branch hole). - Empty threadFileIds: getCodeGeneratedFiles is still called with [] (its own internal early-return handles the empty case); getUserCodeFiles is gated at the call site and stays unscheduled.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Final cutover for the LibreChat ↔ codeapi sandbox file identity. Replaces the magic string
${session_id}/${file_id}?entity_id=...with a typed, discriminatedCodeEnvRef. Companion PRs: codeapi #1455, agents #148. Pre-release lockstep deploy, no legacy aliases retained.Final shape
kinddrives codeapi's sessionKey:<tenant>:<kind>:<id>[:v:<version>]for shared kinds,<tenant>:user:<userId>for user-private (auth context providesuserId).versionis statically required whenkind: 'skill'and statically forbidden otherwise via a discriminated union — the constraint holds at compile time on every consumer, not just the codeapi validator boundary.idis sessionKey-meaningful for'skill'/'agent'; informational-only for'user'(codeapi resolves user identity from auth context). JSDoc spells out the asymmetry.Phases combined in this PR
CodeEnvRef: replaces three separate parsers of the URL-shaped string with a single typed primitive. Newpackages/data-provider/src/codeEnvRef.ts. Schemas:codeEnvRefsubdocument onSkillFileandFile.metadata. Write sites:primeSkillFiles,processCodeOutput,primeFilesreupload.storage_session_idrename: per-filesession_idis the long-lived storage bucket; conflated with the transient execution session. Per-file refs rename tostorage_session_id; top-levelsession_idstays put. Bumps@librechat/agentsto3.1.79-dev.0.kinddiscriminator + tenant-aware cache: explicit per-resource sessionKey shape replacing the emergent "drop user when entity_id is set" behavior. Cross-user-within-tenant sharing for'skill'/'agent'becomes a designed property of the kind switch.entity_ideverywhere (struct, schema sub-docs, write paths, upload form fields). Drops'system'from the kind enum (no emitter ever existed).CODE_ENV_KINDSconst-tuple keeps the runtime list and TS union locked together.CodeEnvRefbecomes a discriminated union. Consumerversionextraction switches fromref.version != nulltoref.kind === 'skill'so the narrowing matches the type definition.uploadCodeEnvFile/batchUploadCodeEnvFilesacceptkind/id/version?and forward them as multipart form fields. Without this, codeapi falls back to user bucketing for every upload and skill-cache invalidation never fires.appendCodeEnvFileIdentitymirrors codeapi's validator (kind from the closed set;versionrequired for'skill'and forbidden otherwise) so a bad caller fails fast on the client.What changed
packages/data-provider/src/codeEnvRef.ts— discriminated union +CODE_ENV_KINDSconst-tuple.metadata.codeEnvRefandSkillFile.codeEnvRefenums tightened to['skill', 'agent', 'user'];entity_idsub-field removed.packages/api/src/agents/skillFiles.ts—primeSkillFileswriteskind: 'skill',id: skill._id,version: skill.version. Pass-through identity onbatchUploadCodeEnvFiles. Cache-hit path readscodeEnvRefdirectly.packages/api/src/agents/handlers.ts—ToolExecuteOptions.batchUploadCodeEnvFilessignature mirrors the new wire shape.api/server/services/Files/Code/process.js—processCodeOutputwriteskind: 'user',id: req.user.id(output bucket always user-scoped, regardless of which skill the execution invoked). Reupload preserveskind/id/version?from the existing ref so a skill-cache-miss reupload doesn't silently demote to user bucket.api/server/services/Files/Code/crud.js—uploadCodeEnvFile/batchUploadCodeEnvFilesthreadkind/id/version?to the multipart form.appendCodeEnvFileIdentityvalidates client-side.api/server/services/Files/process.js— chat-attachment upload useskind: 'user', agent-setup file upload useskind: 'agent'.parseCodeEnvIdentifier/formatCodeEnvIdentifier/resolveCodeEnvRef(no callers; legacy field gone). Migration scriptmigrate-code-env-ref.jsdeleted (pre-release; pre-rename rows lose their cache pointer and self-heal on next prime).Test plan
cd packages/data-provider && npx jest— 1058 passingcd packages/data-schemas && npx jest— 1439 passingcd packages/api && npx jest src/agents— 722 passing (ignoring 2 pre-existing env-dependent summarization e2e failures)cd api && npx jest server/services/Files server/controllers/agents— 423 passingcd api && npx jest server/services/Files/Code— 92 passing (incl. newoutputs are user-scoped regardless of which skill the execution invokedregression andreupload forwards kind/id/version from existing ref)npx tsc --noEmit -p packages/data-{provider,schemas}/tsconfig.json && npx tsc --noEmit -p packages/api/tsconfig.json— cleanDeploy notes
@librechat/agentsbump to3.1.79-dev.0required after agents fix(chatgpt-browser.js): update reverseProxyUrl to use new domain nam… #148 lands and is published.What this enables
Auth bridge work (JWT-based tenant/user identity between LC and codeapi) — codeapi now derives sessionKey purely from
req.codeApiAuthContext.{tenantId, userId}, so the next chapter is replacing the header-asserted user identity with a verified-claim path.