Repository navigation
📍 feat: Show the Current Text Where a Missing Workspace Edit Belongs - #16698
Conversation
Render the current-text excerpt that LibreChat-AI/code-interpreter#300 workers append to a missing edit's EDIT_CONFLICT diagnosis: a strict parse keeps it only when every row is well formed (contiguous numbers matching the stated range, a known mark, a bounded single line), and LibreChat renders it as a numbered block under its edit with a summary of which lines differ. The excerpt passes the file-content policy like a read_file result and reaches only the model; logs keep the excerpt-free message. Also stop rendering every 409 as a text mismatch: a Code API rejection such as WORKSPACE_QUARANTINED now passes through with its own reason, and the worker's repetitive-candidate refusal is recognized.
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. |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Valid Unicode excerpts exceed the transport byte limit and lose their diagnostics before parsing.
Review effort: Balanced
Findings: 1
Open (2)
What changed in this PR
Improves attached-workspace edit failures by showing validated current-text excerpts to the model while keeping file content out of logs.
Changes:
- Parses and renders bounded excerpts and repetitive-match diagnostics.
- Applies file-content filtering and separates model-facing errors from logs.
- Distinguishes edit conflicts from other HTTP 409 rejections.
| File | Description |
|---|---|
| packages/api/src/code/workspace.ts | Extracts codes from complete error bodies. |
| packages/api/src/code/workspace.spec.ts | Tests error-code parsing. |
| packages/api/src/code/edits.ts | Parses, validates, and renders excerpts. |
| packages/api/src/code/edits.spec.ts | Tests excerpt handling and diagnostics. |
| packages/api/src/agents/tools.ts | Documents excerpt-based retry guidance. |
| packages/api/src/agents/handlers.ts | Filters excerpts and separates model and log messages. |
| packages/api/src/agents/handlers.spec.ts | Tests filtering, logging, and rejection handling. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| (upstreamBodyTruncated ? ' [body truncated or incomplete]' : ''), | ||
| ); | ||
| this.name = 'WorkspaceToolHttpError'; | ||
| const parsed = upstreamBodyTruncated ? undefined : parseErrorBody(upstreamBody); |
There was a problem hiding this comment.
Addressed at the producer rather than by raising the reader's cap. LibreChat-AI/code-interpreter#300 now measures excerpts and the whole diagnostic as UTF-8 bytes once JSON-encoded, the same measure as the Code API's {error, code} body:
- each excerpt is capped at 1,600 bytes, dropping trailing rows to fit;
- the message is capped at 3,800 bytes, with excerpts granted only within that and reasons shortened by both length and bytes.
The worker's tests include your example: eight rows of wide CJK lines yield fewer rows, and the full error body stays within 4,096 bytes. It also covers escape-heavy batches and emoji-heavy closest-line quotes.
I kept MAX_ERROR_BODY_BYTES at 4,096. It bounds every upstream error body, not only edit conflicts, and the producer now guarantees the diagnostic fits it. The complete-body guard stays as the safety net for anything that does not.
There was a problem hiding this comment.
Added HTTP-stream regression coverage in c95247b. Unicode diagnostics survive complete 3,800- and 4,096-byte bodies, including split UTF-8 sequences; a 4,097-byte body drops both code and diagnostic. The producer byte budget and 4,096-byte transport cap remain unchanged.
| describeExcerptLines(excerptLinesWith(excerpt, 'changed'), ''), | ||
| describeExcerptLines(excerptLinesWith(excerpt, 'whitespace'), ' only in whitespace'), | ||
| ].filter((part): part is string => part != null); | ||
| return `the closest match is at ${where}${differences.length > 0 ? `, where ${differences.join(' and ')}` : ''}; copy old_text from the current text below`; |
There was a problem hiding this comment.
Agreed, fixed in 6c643b5. The summary no longer says to copy old_text from the block. It now reads "correct old_text against the current text below (it may leave out lines or shorten them with "…"; read_file shows them in full)". The attached edit_file description says the same: fix the edits against any quoted current text, and use read_file for lines it leaves out or shortens. A new formatter test renders a windowed 8-row excerpt with a shortened line, and checks the guidance and the verbatim shortened row.
![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: 862281d71d
ℹ️ 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".
| /** Other 409s (a quarantined workspace) are the Code API's own rejections, not edit misses. */ | ||
| const isEditConflict = error.upstreamCode == null || error.upstreamCode === 'EDIT_CONFLICT'; | ||
| if (error.upstreamStatus === 409 && isEditConflict) { | ||
| throw attachedEditConflict(tc, req, path.filePath, error); |
There was a problem hiding this comment.
When an attached worker returns a 409 with any recognized code other than EDIT_CONFLICT (for example, WORKSPACE_QUARANTINED), this condition bypasses attachedEditConflict and rethrows the original WorkspaceToolHttpError. The outer tool handler then exposes and logs its message, which embeds the worker-controlled upstreamBody; therefore arbitrary diagnostic text, submitted content, or prompt-injection text can reach the model and logs. Preserve the code/status but translate these rejections to an approved safe message and sanitized log metadata rather than forwarding the body.
AGENTS.md reference: AGENTS.md:L122-L128
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f292d03. A 409 whose code is not EDIT_CONFLICT no longer rethrows the original error. It becomes a host-worded message that names only the validated code (upper-case letters, digits and _, at most 64 characters), with recovery guidance for WORKSPACE_QUARANTINED. Its logged body is just {"code":"WORKSPACE_QUARANTINED"}. The new test sends an injected quarantine body and asserts that neither the model message nor the logs contain it.
For context: the Code API maps only EDIT_CONFLICT worker settlements to 409, so these bodies come from its own store errors rather than from workers. Not forwarding them is still the safer default.
| const MAX_EXCERPT_LINES = 8; | ||
| /** The worker shortens each quoted line to 160 characters plus an ellipsis. */ | ||
| const MAX_EXCERPT_LINE_CHARS = 161; |
There was a problem hiding this comment.
The new grammar accepts up to eight 161-character lines per excerpt, but readErrorBody in workspace.ts caps the entire upstream error response at 4096 bytes and marks an over-limit response truncated, after which WorkspaceToolHttpError deliberately does not populate editConflict. A single maximal excerpt containing multibyte source text can exceed that cap, and three maximal ASCII excerpts already exceed it, so valid worker diagnostics supported by this parser are discarded on the real HTTP path and the model receives only generic retry guidance. Bound excerpts using the transport's UTF-8 byte budget or raise the error-body limit consistently.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed at the producer in LibreChat-AI/code-interpreter#300 (b1a197e), which owns the message budget. Sizes are now measured as UTF-8 bytes of the JSON-encoded string, the same measure the Code API's {error, code} body uses:
- each excerpt is capped at 1,600 bytes, dropping trailing rows to fit;
- a batch grants excerpts only while the whole message stays within 3,800 bytes;
- a single edit drops its excerpt rather than exceed that.
This leaves room for the wrapper under MAX_ERROR_BODY_BYTES (4,096). New worker tests check that the full error body stays within 4,096 bytes for multibyte source lines and for escape-heavy batch excerpts.
The parser's per-row limits here stay as they are. They bound what LibreChat accepts, not what fits the transport, and excerpts that fit the body always satisfy them.
…eir Body A 409 whose code is not EDIT_CONFLICT (a quarantined workspace) now gets a host-worded message naming only its validated code, with recovery guidance for WORKSPACE_QUARANTINED, and logs keep a body of just that code.
|
Codex Review: Didn't find any major issues. Can't wait for the next one! 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". |
codegraph-librechat
Bot
added
the
🗺️ Backend Platform
|
Codex Review: Didn't find any major issues. You're on a roll. 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". |
A worker excerpt can window a long region and shorten wide lines with an ellipsis, so the guidance now asks the model to correct old_text against it and use read_file for lines it leaves out or shortens, here and in the attached edit_file descriptions.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c643b532f
ℹ️ 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 (!excerpts || filteredFileResult(tc, req, filePath, excerpts) != null) { | ||
| return settle(message); | ||
| } | ||
| return settle(message, formatEditConflict(path, report, true)); |
There was a problem hiding this comment.
When a selected workspace advertises edit_file without read_file—a supported configuration exercised by agents/tools.spec.ts—this still returns worker-supplied current file text after a deliberately failing edit. That turns edit_file into a read oracle and bypasses the operation check enforced by handleWorkspaceFileRead; suppress excerpts unless selectedWorkspaceId(codeExecutionContext, 'read_file') also succeeds.
AGENTS.md reference: AGENTS.md:L186-L189
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 11a8aa8. Excerpts are now quoted only when selectedWorkspaceId(codeExecutionContext, 'read_file') succeeds. Otherwise the model gets the excerpt-free message, the same as when the file-content policy blocks the excerpt. This mirrors the worker, which already replaces the whole diagnostic for workspaces without read_file/preview_edit; LibreChat now enforces the same rule against its own view of the selected workspace's operations. A new handler test drops read_file from the workspace operations and asserts that no file text is returned.
A workspace that offers edit_file without read_file now never receives the worker's current-text excerpt, so a failing edit cannot read file content the operation policy withholds.
|
@codex review the latest head, final review |
|
Codex Review: Didn't find any major issues. Keep them coming! 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". |
|
Head: c95247b. Conflicts resolved against dev; path-rejection guidance, policy-checked excerpts, and sanitized 409s preserved. Added Unicode HTTP-stream boundary and tool-output policy regressions. Checks: 973 focused tests passed, 1 skipped; API typecheck and worker/declaration build passed; PR-scoped static checks passed. Lighthouse passed in CI; local browser run was blocked by missing Chromium. Final independent review of this exact head: no findings. Backend CI is green; remaining E2E and runtime smoke checks are running. |
Summary
When an
edit_filein an attached (BYOM) workspace misses, LibreChat tells the model which edit failed and a line number at best. #16520 deliberately dropped the one file excerpt workers sent, and nothing else let the model see what the file actually says there. So the usual next step is aread_fileof the whole file, then another attempt. LibreChat-AI/code-interpreter#300 makes the worker quote the current text of the region the edit most likely meant, marked line by line. This PR renders that excerpt for the model, keeps it out of the logs, and fixes a mislabeled 409.Depends on LibreChat-AI/code-interpreter#300 for the excerpts. Merging this first is safe: without that worker change, the messages are unchanged.
Evidence (demo.librechat.ai, last 7 days)
edit_filecalls; 90 failed withEDIT_CONFLICT(3.4%).WORKSPACE_QUARANTINED409 that LibreChat called a text mismatch.tolerant_match, and LibreChat sendsmatching: 'tolerant'by default (🎯 feat: Tolerant Workspace Edits with Host-Rendered Conflict Diagnostics #16520). Whitespace drift therefore no longer fails, and ambiguous matches already list every location.old_textwith the next successful edit to the same file:read_fileof the same file before the retry. The stale-content case is the one this PR targets.How it works
Excerpt parsing (
code/edits.ts). The worker's excerpt is always its last hint:parseEditConflictkeeps it only if every row is well formed:~or!;Otherwise the excerpt is dropped and the other hints are kept. A complete JSON-quoted closest-line snippet is now skipped, so an excerpt after it can be read. The snippet text itself is still never kept.
Rendering (
formatEditConflict(path, report, withExcerpts)). With excerpts, each missing edit says which lines differ, then shows a block numbered likeread_fileoutput:The block is presented as partial context: the worker shows at most 8 rows and shortens wide lines, so the model is pointed to
read_filefor anything left out. In a batch, each block follows its ownEdit N:line. Without excerpts the output is exactly what #16520 produced.Trust and logging (
agents/handlers.ts). #16520 forwards no free text from a worker, and that does not change: every sentence is still LibreChat's own, built from parsed facts. The excerpt is the exception, because it is file content from the file being edited, which the same worker already returns throughread_file. It is handled like aread_fileresult:read_file, so a failing edit can never reveal file text that the operation policy withholds. The worker applies the same rule on its side.filteredFileResult, the file-content policy. If the policy would block it, the model gets the excerpt-free message.errorMessage, so the tool-output filter still sees it.AttachedEditRejectionErrorcarries the log-safe text inmessageand the excerpt version inmodelMessage.getSafeToolErrorgives the modelmodelMessageand keepserrorMessagein the log context excerpt-free. The loggedupstreamBodystays the sanitized{"code":"EDIT_CONFLICT"}.Other 409s. A Code API rejection such as
WORKSPACE_QUARANTINEDused to be rendered as "the requested text did not match exactly once; re-read the file and retry". Re-reading cannot fix a quarantined workspace.WorkspaceToolHttpErrornow exposesupstreamCodefrom a complete JSON body, restricted to upper-case code names. OnlyEDIT_CONFLICT, or a body without a readable code, goes through the conflict path. Any other 409 gets a host-worded message that names only its code, plus recovery guidance forWORKSPACE_QUARANTINED("must be reset on its machine before edits can apply, so retrying will not help"). Its body is never forwarded or logged.Also:
edit_filedescriptions mention the quoted text.Compatibility. Excerpts appear only with workers that send them (code-interpreter#300). Older workers produce the same messages as before. Older LibreChat builds drop the excerpt hint and render what they render today.
Type of change
Testing
Tested environments/configuration:
Automated tests:
packages/api: focused code and agent-handler suites, run in-band with sandbox-safe MongoDB sockets: 973 passed, 1 skipped. New coverage:withExcerpts;upstreamCodeparsing (complete bodies only, code names only);errorMessageandupstreamBodycontain no file text; a file-content policy match, or a workspace withoutread_file, drops the excerpt; aWORKSPACE_QUARANTINED409 is named by its code, with neither the model message nor the logs carrying its body.npx tsc --noEmitinpackages/api;npm run build:api;npm run static-checks -- --against origin/dev(ESLint, Prettier, import sorting, manifest validation, circular dependencies).Screenshots / recordings
Not applicable. No UI change; only the text of a failed edit's tool result changes.