Repository navigation
📎 feat: Copy Chat Attachments into Attached Workspaces - #17376
Conversation
A chat on an attached workspace now copies the user's own attachments into .librechat/attachments/ before the model runs, through the worker's import_file operation, and tells the model where they are. A worker without the operation, a deny file-write policy, or a copy that fails or runs past its bound leaves a plain note saying the file is not on this machine and why.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 7d10161338
ℹ️ 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".
| if (digest) { | ||
| update['metadata.workspaceImports.digest'] = digest; | ||
| } | ||
| const result = await File.updateOne({ file_id, user }, { $set: update }); |
There was a problem hiding this comment.
When one user has records with the same file_id in multiple tenants, this owner-only filter can update an arbitrary tenant's record, causing cross-tenant metadata mutation while the active tenant's file is imported again on later turns. Thread the authenticated file's tenantId through WorkspaceImportRecord and include tenantId: tenantId ?? null in this update filter.
AGENTS.md reference: AGENTS.md:L187-L190
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 6643e64: the record carries the file's tenantId and the update matches tenantId: tenantId ?? null, with a spec for a same-user record in another tenant.
| await primeWorkspaceAttachments({ | ||
| agents: [primaryConfig, ...agentConfigs.values()], | ||
| toolContexts: agentToolContexts, |
There was a problem hiding this comment.
For an explicit lazy subagent selected after the run starts, initializeLoadedSubagent initializes its tools only after this one-time call has completed, and that config is never added to agentConfigs. Consequently its codeContextEntries are never imported into its selected workspace and its model retains the stale programmatic-only file note; invoke attachment priming as part of lazy initialization as well.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 6643e64: priming goes through one per-turn primer (createWorkspaceAttachmentPrimer, WeakSet of primed configs). initializeLoadedSubagent primes each config it initializes, eager or lazy, and the turn-level call skips configs already primed.
| /** The whole import phase, provisioning included, never holds a turn longer than this. */ | ||
| export const WORKSPACE_ATTACHMENT_IMPORT_TIMEOUT_MS = 10_000; | ||
| const WORKSPACE_ATTACHMENT_IMPORT_CONCURRENCY = 3; |
There was a problem hiding this comment.
Deployments whose storage download, provisioning, or worker queue regularly exceeds 10 seconds will always report otherwise valid attachments as timed out, but this newly introduced timeout cannot be tuned. Add a configSchema field with a 10-second default and pass the resolved value into the attachment dependencies rather than fixing it in code.
AGENTS.md reference: AGENTS.md:L91-L93
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 6643e64: endpoints.agents.workspaceAttachments.timeoutMs (1-120 s, default 10 s) is threaded into the attachment deps.
| const timer = setTimeout(() => deadline.abort(), timeoutMs); | ||
| const abortOnTurn = () => deadline.abort(signal?.reason); | ||
| signal?.addEventListener('abort', abortOnTurn, { once: true }); |
There was a problem hiding this comment.
If the run is canceled while earlier initialization is awaiting and this function is entered with an already-aborted signal, adding the listener does not invoke it, so the independent deadline remains live and primed attachments can still be digested and written for up to 10 seconds before the final throwIfAborted. Check signal.aborted and abort the deadline before starting any import work.
AGENTS.md reference: AGENTS.md:L50-L53
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 6643e64: primeWorkspaceAttachments throws on an already-aborted turn signal before any digest or import starts.
| if (target.context.codeWorkspace?.operations.includes('write_file') !== true) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
When a worker advertises import_file but not write_file, the attachment policy still permits imports while this early return prevents creation of the exclusion file. The copied user files then appear as ordinary untracked repository content and can be accidentally committed; either require both capabilities before importing or make import_file guarantee an equivalent repository exclusion.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 6643e64: import requires both import_file and write_file; without write_file the file gets the 'cannot receive attachments yet' note.
| JSON.stringify([ | ||
| conversationId, | ||
| context.environmentId ?? workspace.environmentId, | ||
| workspace.workspaceId, | ||
| workspace.workspaceInstanceId ?? null, | ||
| ]), |
There was a problem hiding this comment.
If an environment ID is retained while its Code API host, bridge worker, or advertised environment fingerprint changes, this key remains identical even though it now addresses a replacement workspace. Existing per-file metadata will therefore satisfy the digest shortcut and skip import_file, while the model is told that a path exists on the new machine; incorporate the execution route and workspace environment fingerprint into the target identity.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 6643e64: the target key now includes the normalized Code API base URL and the bridge worker id, so a replacement worker under the same environment id gets the copy again. The environment fingerprint describes the repo setup, not the machine, so it stays out.
| try { | ||
| await deps.recordImport({ | ||
| file_id: file.file_id, | ||
| user: deps.userId, | ||
| key, | ||
| target: copy, | ||
| ...(digest ? { digest } : {}), | ||
| }); |
There was a problem hiding this comment.
If MongoDB stalls after the worker has copied a file, this awaited recordImport call receives no abort signal and is not raced against the deadline, so the supposedly 10-second-bounded priming phase can block the entire turn until the database driver's much longer timeout. Make this optional bookkeeping abortable or stop awaiting it once the attachment deadline expires.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 6643e64: the bookkeeping write is raced against the deadline; past it the turn continues and the write lands when it can.
Match the tenant when recording a workspace copy, require write_file so the folder can always ignore itself, key copies by the live Code API route and worker, keep a digest when a copy fails so it is not hashed again, stop waiting on bookkeeping at the deadline, refuse to start on a cancelled turn, prime lazily initialized subagents through one per-turn primer, and make the budget configurable as endpoints.agents.workspaceAttachments.timeoutMs.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6643e6401a
ℹ️ 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".
| if (recorded?.sha256 === digest.sha256) { | ||
| return { status: 'imported', path: recorded.path }; | ||
| } | ||
| void ensureGitignore(); |
There was a problem hiding this comment.
When .librechat/attachments/.gitignore already exists without a rule that ignores the directory contents (the added test uses only # mine), the create-only write returns a conflict that writeAttachmentsGitignore treats as success, while this line proceeds with the import regardless. The uploaded attachment then appears as ordinary untracked repository content and can be committed or pushed accidentally; only import after confirming an effective ignore rule, or report the attachment as unavailable when that cannot be guaranteed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 95b92c7: every copy now waits on the ignore check. The .gitignore is created with *; an existing one is read and accepted only when it ignores the whole folder (*, /*, **, /**, no negations). Otherwise, or when the write or read fails, the file is reported as unavailable with how to fix it.
| const timer = setTimeout(() => deadline.abort(), timeoutMs); | ||
| signal?.throwIfAborted(); |
There was a problem hiding this comment.
When this function is entered with an already-aborted turn signal, the newly added throwIfAborted runs only after this timer is created and it throws outside the try/finally, so the timer is never cleared. Each such cancellation retains its controller and callback for the configured timeout—up to 120 seconds—which can accumulate during cancellation bursts; check the signal before creating the timer or ensure this path also clears it.
AGENTS.md reference: AGENTS.md:L45-L48
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 95b92c7: the cancellation check runs before the deadline controller and timer are created.
| toolContext: buildAgentToolContext({ agent: routedAgent, config }), | ||
| }); | ||
| endpointTokenConfigByAgentId.set(agentId, config.endpointTokenConfig); | ||
| await primeAttachments([config], context.signal); |
There was a problem hiding this comment.
When the same lazy subagent is initialized concurrently for multiple routed executions while an attachment still needs provisioning, subagentCodeRouting.attach keeps later tool contexts under their execution IDs rather than replacing agentToolContexts, but this call gives the primer only the config and signal. provisionQueued subsequently calls provisionFiles with the agent ID and no execution context, so it can provision the first execution's state while the later config retains no source reference and reports its attachment as unavailable; pass the exact routed tool/execution context through attachment priming.
AGENTS.md reference: AGENTS.md:L45-L48
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 95b92c7: initializeLoadedSubagent primes the config with the exact tool context it attached, through a provisioning callback bound to a single-entry map of that context, so concurrent routed executions each provision their own state.
Gate every copy on .librechat/attachments/.gitignore ignoring the whole folder: create it with *, and accept an existing one only when it already ignores everything, otherwise report the file as unavailable with how to fix it. Prime a lazily routed subagent against its own tool context with a provisioning callback bound to it, and check for a cancelled turn before arming the deadline timer.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 95b92c714d
ℹ️ 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".
| if (gitignoreTargets.has(cacheKey)) { | ||
| return null; |
There was a problem hiding this comment.
When a user removes or changes .librechat/attachments/.gitignore after the first successful import while this server process remains alive, this process-global fast path returns success without another workspace call, so later attachments are copied into a directory that may now expose them as untracked repository content. Cache only concurrent in-flight checks, or revalidate the ignore rule before each new copy.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8eba820: the process-wide ignore cache is gone. The folder is checked once per turn (create-only write, then a read of an existing file), shared by the agents of that turn, so an edit to the file is seen by the next turn that copies anything.
| if (existing.operation !== 'read_file' || !ignoresWholeFolder(existing.content)) { | ||
| return 'not_ignored'; |
There was a problem hiding this comment.
When the existing .gitignore exceeds 200 lines, this validates only the returned prefix and ignores existing.truncated; for example, * near the top and !keep.txt after line 200 makes keep.txt visible to Git while this check permits importing an attachment with that name. Reject truncated results or continue reading the entire file before deciding that the folder is safely ignored.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8eba820: a truncated read counts as not ignored.
| if (target.context.codeWorkspace?.operations.includes('read_file') !== true) { | ||
| return 'not_ignored'; |
There was a problem hiding this comment.
When two agents concurrently import distinct files into the same empty workspace and the worker advertises import_file and write_file but not read_file, both issue the create-only marker write; the losing request receives a conflict and this branch reports not_ignored, even though the winning request just created the valid marker. Share one in-flight ignore check per target so every concurrent importer observes the successful creation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8eba820: the ignore check is one shared promise per (route, worker, workspace, checkout) for the whole turn, so concurrent importers all wait on the same creation.
| }); | ||
| } catch (error) { | ||
| if (classifyImportFailure(error) !== 'conflict') { | ||
| logger.warn('[workspaceAttachments] Could not create the attachments .gitignore', error); |
There was a problem hiding this comment.
When the Code API rejects this write, WorkspaceToolHttpError retains the raw upstreamBody, and passing the whole error to the logger can persist arbitrary worker response content, including workspace details. Log only classified or sanitized metadata such as getSafeErrorMetadata(error); the same treatment is needed at the other new catch sites that log raw errors.
AGENTS.md reference: AGENTS.md:L123-L129
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8eba820: every new log site passes getSafeErrorMetadata(error) instead of the raw error.
| const pending = ordered.map((candidate, index) => ({ | ||
| candidate, | ||
| name: | ||
| recordedNames[index] ?? claimAttachmentName(taken, toAttachmentName(candidate.file.filename)), |
There was a problem hiding this comment.
When an uploaded image is converted during storage while its file record keeps the original filename, this names the workspace copy with the original extension even though the source bytes and file.type describe the converted format. The existing resolveSandboxFilename contract in packages/data-provider/src/file-config.ts deliberately renames sandbox copies for decoder correctness, so this path can produce PNG or WebP bytes named like the original HEIC/BMP file and make workspace tools fail to decode it; derive the import name with the same MIME-aware rule.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8eba820: the workspace name is derived from resolveSandboxFilename(filename, type) before sanitizing, so a converted image is named for its stored type and the note says 'uploaded as' the original.
| export const WORKSPACE_ATTACHMENTS_DIRECTORY = '.librechat/attachments'; | ||
| /** The whole import phase, provisioning included, never holds a turn longer than this. */ | ||
| export const WORKSPACE_ATTACHMENT_IMPORT_TIMEOUT_MS = 10_000; | ||
| const WORKSPACE_ATTACHMENT_IMPORT_CONCURRENCY = 3; |
There was a problem hiding this comment.
On deployments whose attached workers require lower request pressure—or can safely support more throughput—this fixed value cannot be tuned, and it is applied independently per concurrently primed agent, so the actual workspace request fan-out can greatly exceed three. Add a workspaceAttachments concurrency field with a default of three and thread it into mapConcurrent.
AGENTS.md reference: AGENTS.md:L91-L93
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8eba820: endpoints.agents.workspaceAttachments.concurrency (1-16, default 3) sets one limiter for the whole turn, shared across agents and later subagent priming.
| const result = await File.updateOne( | ||
| { file_id, user, tenantId: tenantId ?? null }, | ||
| { $set: update }, | ||
| ); |
There was a problem hiding this comment.
When a project file is copied into an attached workspace, this metadata-only updateOne uses Mongoose's default timestamp behavior and advances updatedAt even though the file content did not change. packages/api/src/projects/resources.ts includes updatedAt in the canonical project-resource version, so an approval or resumed turn whose snapshot predates attachment priming can reject the unchanged resource as a new version; pass { timestamps: false } for this bookkeeping write.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8eba820: the bookkeeping updateOne passes { timestamps: false }.
…uest Cap Drop the process-wide ignore cache and check the folder once per turn, shared by every agent, rejecting a truncated read. Cap workspace requests per turn across agents (workspaceAttachments.concurrency, default 3), name converted images for their stored type, log only safe error metadata, and keep the bookkeeping write from advancing updatedAt.
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ 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". |
dev and this branch both add to the same lines of the file methods module. Take dev's version here so the merge of dev that follows is clean; the next commit adds recordFileWorkspaceImport back beside dev's new methods.
Bring recordFileWorkspaceImport back beside dev's new file methods after the merge. Drop well-formed operation names this build does not know from a worker's capabilities, globally and per workspace, instead of rejecting the whole status, so workers can turn on new operations such as import_file by default. Known names keep their strict checks, malformed lists are still refused, and a worker offering only unknown operations offers no workspace tools rather than failing.
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ 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". |
Pull Request
Summary
When a chat runs on an attached personal-machine workspace, files the user uploads never reach that workspace. The code-file note told the model the files were only readable by programmatic Bash at
$LIBRECHAT_CODE_DATA_DIR/..., so an agent without that tool (most of them) could not open them at all, and one that triedbash_toolwith the variable found nothing. A user who attached a release-prep ZIP on a workspace hit exactly this: upload, then "the file is not there".Before the model runs, a turn on an attached workspace with a selected workspace now copies every chat file the model would be told about into
.librechat/attachments/<name>through the worker's newimport_fileoperation, and the code-file note lists those workspace paths, so every workspace tool (shell, read, search) can open them, binaries included. The upload is the consent: the copy never goes through the tool-approval prompt, but adenyfile-write policy on the machine is respected. Only the requesting user's own files are copied, and only into.librechat/attachments/, which must ignore itself: its.gitignoreis created with*and never replaced, and a copy goes ahead only once that file, or one the user already had, ignores the whole folder.Anything that is not copied gets a plain line instead of a path the agent cannot use: the worker cannot receive attachments yet, file writes are not allowed in this workspace, the existing ignore file does not cover the folder, it is the agent's own file, it exceeds the machine's size limit, or the copy did not finish in time. The
$LIBRECHAT_CODE_DATA_DIRpath is still offered, but only to an agent that actually has the programmatic Bash tool.Depends on LibreChat-AI/code-interpreter#348 for the
import_fileoperation. The code bridge is experimental, so its new features are on by default: there is no LibreChat setting to enable the copy. It runs whenever the selected workspace advertisesimport_fileandwrite_file, which workers do by default once #348 ships. Until then, the only visible change is the clearer "not available on this machine" note.This PR also makes LibreChat forward-compatible with newer workers. A worker status that lists a workspace operation this build does not know (globally or for one workspace) used to be rejected whole, which made the machine unusable. Well-formed unknown names are now dropped and the known ones kept, with their strict checks unchanged; a malformed list (non-strings, bad names, duplicates, more than 64 entries) is still refused, and a worker offering only unknown operations simply offers no workspace tools.
How it works
The phase runs once per agent per turn, before the run starts for agents initialized with the turn and at initialization for a lazily selected subagent, and is bounded by
endpoints.agents.workspaceAttachments.timeoutMs(default 10 seconds, 1 to 120 seconds); whatever is unfinished falls back to the note and is retried next turn. Workspace requests for the whole turn, across every agent, are capped byendpoints.agents.workspaceAttachments.concurrency(default 3).Repeat turns cost nothing extra on the hot path: the marker and the digest ride on the file records the turn has already loaded, so a recorded copy is skipped without a database read or a workspace call, and the only write is one owner- and tenant-scoped
$setper newly copied or newly hashed file. That write is bookkeeping: it does not advanceupdatedAt, and past the deadline the turn stops waiting for it. The worker's own digest check (unchanged: true) makes a resend harmless when no marker exists yet.Names follow the stored type the way sandbox copies do (a converted image is named for what it holds), then are sanitized to a portable single segment (no separators, controls, reserved characters, leading dots or reserved Windows device names; long names keep their extension) and de-duplicated case-insensitively in upload order as
name (2).ext. A copy recorded in an earlier turn keeps its name. A different file already holding the name (409 EDIT_CONFLICT) moves to the next free name, up to three times. A conversation's isolated checkout (workspaceInstanceId) is targeted the same way the workspace tools target it, with its own marker.Queued uploads, the common case for a file attached in this turn, have no Code API object until something provisions them. The importer calls the same provisioning callback the first code tool would; refs that arrive in time seed the run's sessions (
primedCodeFiles), and late ones are handed to that first tool call throughpendingProvisionedCodeFiles, exactly as lazily provisioned files are.Type of change
Testing
Tested environments/configuration:
mongodb-memory-server; worker contract exercised through an in-memory Code API and worker that implementsimport_fileand create-onlywrite_fileas specified in the code-interpreter PR.Automated tests:
packages/api/src/code/attachments.spec.ts(new): capability present imports and lists workspace paths; capability absent sends nothing and explains why; deny policy skips while anaskpolicy imports; worker-sideWRITE_DISABLED; same-digest file already present; recorded copy not resent and not re-hashed; name conflict moves toname (2).ext; same-named uploads in upload order;.gitignorewritten once per workspace, an existing one that ignores the folder left alone, one that does not (or was changed later, or was read truncated) blocking every copy; converted images named for their stored type; one request cap across agents; isolated checkout targeting with a separate marker; agent-owned and other users' files never copied; timeout fallback within the bound; sandbox routes untouched; queued uploads provisioned, copied, and kept out of the queued note; late provisioning refs handed to the first tool call; one copy for two agents on the same workspace; one priming per agent including a later subagent; no import withoutwrite_file; nothing started on a cancelled turn; a stalled metadata write does not hold the turn; a replacement worker under the same environment id gets the copy again; a refused file is not hashed twice; filename sanitization;import_filerequest and result validation, including nanoid ids that start with a symbol.packages/api/src/code/bridge.spec.tsandcapabilities.spec.ts: a status listing unknown operations (worker-level, per workspace, legacy list) still yields the workspace with its known operations, includingimport_file; malformed lists and a root claiming a known operation the worker lacks are still rejected; a worker offering only unknown operations has no workspace tools.packages/data-schemas/src/methods/file.spec.ts:recordFileWorkspaceImportkeeps sibling targets, the digest and other metadata, refuses another user's record and the same user's record in another tenant, rejects a non-hex key.packages/data-provider/src/config.spec.ts: theworkspaceAttachmentstimeoutMsandconcurrencydefaults and bounds.packages/api/src/files/provision/service.spec.ts:digestFilehashes the stored bytes, returns null for a source without a stream, and stops on abort.api/server/services/Files/Code/process.spec.js:primeFilesreturns structured entries only on the programmatic route.api/server/services/Endpoints/agents/initialize.spec.js: the attached-workspace note is rewritten before the client is built.npx tsc --noEmitinpackages/api,packages/data-schemasandpackages/data-provider; ESLint, Prettier and import sorting on every touched file.Screenshots / recordings
No user-facing change.
Risk / compatibility
import_fileis added toCODE_WORKSPACE_OPERATIONS. Deployments without this PR reject a worker status that lists an operation they do not know, so they need it before workers advertiseimport_fileby default. With it, LibreChat drops unknown operation names instead (packages/api/src/code/bridge.ts), so later worker operations no longer require this ordering. The vendored@librechat/codepackage holds only the browser-enrollment protocol and plays no part in this validation. The request and result types are local topackages/api/src/code/workspace.ts; the vendored@librechat/codepackage carries only the browser-enrollment protocol today, and the vendor refresh follows the code-interpreter PR.File.metadata.workspaceImportsis a new optionalMixedfield, written only throughrecordFileWorkspaceImport, which accepts only hex target keys and matches the owner. No migration.endpoints.agents.workspaceAttachments.timeoutMs(default 10 seconds). Later turns skip recorded copies without I/O.import_fileandwrite_file, so the folder can always ignore itself.Checklist