🏋️ fix: Bound Host File Edits Outside the API Event Loop - #16677
Conversation
|
Head: E adds cumulative edit-work limits and a bounded cancellable worker pool for host skill/sandbox edits. Attached edits retain their route.
|
codegraph-librechat
Bot
added
the
🗺️ Backend Platform
|
Head: Host skill/sandbox edits have cumulative byte/occurrence limits, a server-side edit cap, and bounded off-thread processing. Attached edits retain their route.
|
|
Head: Synced with current dev without rewriting history. The configuration conflict retains both host-edit limits and subagent-activity policy. Security behavior is unchanged from the preceding head. E-R1 (P2, late deadline acceptance) is fixed in Current-head local verification and CI are running. A dedicated frozen-head review worktree is being prepared with its own dependencies so the next review can execute runtime checks. Earlier-head checks/review do not certify this synchronized head. No deployment or HTTP/model-provider exploit is claimed. |
|
Head: E bounds host skill/sandbox edit work off the API event loop. Attached-worker edits retain their route.
No merge or deployment performed. |
|
@codex review the latest head, final review |
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: 4ce4507972
ℹ️ 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".
| .int() | ||
| .min(1024) | ||
| .max(256 * 1024 * 1024) | ||
| .default(32 * 1024 * 1024), |
There was a problem hiding this comment.
When hostFileEdits is omitted, a single exact edit of an ASCII file larger than roughly 8 MiB but within the existing 10 MiB authoring limit is rejected: the matcher charges the working file once before matching, once during exact matching, and roughly twice during reconstruction, so a 9 MiB file consumes about 36 MiB against this 32 MiB default. Such files were accepted previously and still satisfy MAX_AUTHORING_BYTES; raise the default or adjust the accounting so the ordinary single-edit path remains supported.
AGENTS.md reference: AGENTS.md:L90-L92
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d97c7ca5d2b949fdb4ec757d79327a3380870436. The default cumulative work budget is 64 MiB, preserving ordinary exact edits through the existing 10 MiB authoring limit. Occurrence, batch-count, output-size, deadline, and concurrency bounds remain enforced. New real-worker 9/10 MiB regressions and a real 10 MiB sandbox-handler edit pass; amplification and configured lower-budget rejection tests still pass.
There was a problem hiding this comment.
The compatibility sweep found a residual case when old_text spans the full file. Fixed in 0f283deda29175e02fbb499a2fa09c40f5863d67: admission bounds actual UTF-16 clone units, and worker accounting reuses each edit's byte measurements without redundant scan charges. Default remains 64 MiB. Full 9/10 MiB ASCII and 10 MiB UTF-8 contexts, full-context replace_all/contraction, and real sandbox persistence pass; cumulative work limits and amplification rejection remain green.
| } catch (error) { | ||
| if (signal?.aborted) throw error; | ||
| return errorResult(tc, error instanceof Error ? error.message : 'Failed to edit file'); |
There was a problem hiding this comment.
If worker initialization throws synchronously—for example because thread creation fails under resource pressure—this catch returns the raw operational error.message as the tool result; the equivalent skill-file path does the same. Unlike HostEditError, such messages are not controlled and may expose absolute paths or runtime diagnostics to a persisted, user-visible surface, so only known safe edit errors should be forwarded and all other failures should use the existing generic message.
AGENTS.md reference: AGENTS.md:L122-L128
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d97c7ca5d2b949fdb4ec757d79327a3380870436. Only host-controlled HostEditError diagnostics are forwarded. Skill and sandbox handlers now use a generic message for unexpected failures, and synchronous worker acquisition failures are sanitized before admission is charged. Regressions cover constructor failure/capacity recovery, both skill write paths, synchronous/asynchronous sandbox failures, no diagnostic disclosure, and no writes. Safe edit feedback remains unchanged.
|
Head: Both Codex P2 findings are fixed: the default work budget supports ordinary exact edits through the 10 MiB authoring limit, and uncontrolled worker startup failures cannot reach tool results. A boundary sweep rechecked admission, budgets, cancellation, deadlines, controlled diagnostics, and final-write fences.
|
|
@codex review the latest head, final review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d97c7ca5d2
ℹ️ 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".
| const ctx = sandboxSessionContext(tc, sandboxContext); | ||
| let writeResult: Awaited<ReturnType<NonNullable<ToolExecuteOptions['writeSandboxFile']>>>; | ||
| try { | ||
| signal?.throwIfAborted(); |
There was a problem hiding this comment.
When the run signal aborts after host edit processing completes but before the sandbox write starts, this check throws an AbortError inside the surrounding try, so the catch immediately converts it into an ordinary tool error. The run cancellation machinery therefore cannot recognize the cancellation; move the check outside this catch or rethrow abort errors before translating write failures.
AGENTS.md reference: AGENTS.md:L45-L50
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 96164f19dc865c2e646d013358eb48b98da9478e. The final pre-write abort check is outside the write-failure catch; an in-flight write AbortError with an aborted run signal is rethrown before error translation. Both real-handler regressions fail on the previous head and pass now: pre-write cancellation dispatches no write, and both cases reach the run-abort classification without a success artifact or ordinary write-error warning. Normal write failures retain their existing behavior.
|
Head: The residual full-context P2 finding is fixed. Admission bounds UTF-16 clone bytes; the worker separately accounts UTF-8 input, matching and reconstruction. Per-edit byte measurements are reused rather than recharging repeated measurements. The 64 MiB default is unchanged. Configured lower-budget and amplification rejection remain enforced.
|
|
@codex review the latest head, final review |
|
Codex Review: Didn't find any major issues. Delightful! 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: Exact-head CI is fully green: 43 passed, 2 skipped, no failed/pending checks. All five reported P2 findings have fixing commits and regression coverage recorded above. The user-requested independent runtime retry is blocked before execution. Git repaired the mandatory reviewer-owned linked worktree, but the worker still rejects its No source changes, merge, or deployment. Restore review-lane routing before another runtime attempt. |
2 participants
Summary
Exact
replace_allbatches introduced in #16520 can amplify tiny skill/sandbox files and repeatedly scan/rebuild large intermediate strings on the API event loop. At base73d704656e39, the reported 54-edit sequence took 11,525 ms and prevented a scheduled timer from firing. The tolerant-window optimization in #16562 does not bound this exact-replacement workload.Run host-side matching in a bounded reusable worker pool. Enforce cumulative scanned/reconstructed bytes, cumulative occurrence processing, edit count, intermediate output size, and a job deadline. Busy, over-budget, cancelled, or failed jobs write nothing. Attached-workspace edits retain their worker route and existing guarantees.
How It Works
The pool admits two jobs per API process by default, reuses idle threads, and retires idle or cancelled workers. The job deadline uses a monotonic clock and is checked before accepting replies, independent of timer ordering. Deadline/cancellation termination releases admission only after the worker exits. No application services, storage, or credentials are loaded in the matcher worker. The existing matcher precedence, UTF-16 offsets, ambiguity handling, and ordered replacements are preserved. Admission bounds UTF-16 clone bytes; matching separately accounts UTF-8 input and each scan/reconstruction pass, reusing per-edit size measurements.
Configuration
Optional
endpoints.agents.hostFileEditscontrols host skill/non-attached-sandbox processing:maxEditsmaxWorkBytesmaxOccurrencestimeoutMsmaxConcurrentDefaults apply when omitted. Occurrences count processing in each pass, not only unique replacements. Limits cannot be disabled; validated upper bounds remain in force. This intentionally rejects formerly unbounded work. Update all API replicas before enabling the new strict configuration field. No database migration or Code API change.
Verification
At head
96164f19dc865c2e646d013358eb48b98da9478e, 577 focused API tests, 371 configuration tests, and 29 legacy image-tool tests passed. API/data-provider typechecks, real data-provider/API builds, and touched-file static checks passed. Current-head independent review and CI status are recorded in the head comment.Regressions cover compact amplification without timer starvation or writes, configured byte/occurrence limits, cancellation/deadlines and capacity recovery, final-write fences, pre-write/in-flight sandbox abort propagation, matching semantics, 9/10 MiB omitted-default compatibility with short and full matching context, multibyte text, full-context replace_all/contraction, actual 10 MiB sandbox persistence, deterministic work accounting, synchronous thread-creation failure, and safe errors through both skill and sandbox paths. Only host-controlled edit diagnostics are surfaced; unexpected failures use a generic message.
All five reported P2 findings are fixed. No findings were rejected. Independent review of
96164f19dc865c2e646d013358eb48b98da9478efound no new defects and passed 18 matcher probes, 2,000 differential cases, and three extracted sandbox-write probes. It remains incomplete: production worker/handler/configuration/persistence suites were unavailable in the isolated lane. Earlier incomplete reviews are not counted as clean reviews of this head. Exact-head CI Lighthouse, integrations, unit tests, builds, static checks and runtime smoke passed; all exact-head CI is now green (43 passed, 2 skipped) as of 2026-10-03 00:42 UTC.Local Lighthouse is blocked by missing Chromium after a scratch-only Mongo socket workaround. Full local suites, config-migration/unused-package full gates, HTTP/model-provider exploit verification, and deployed-load benchmarks were not run. No UI layout changes.
Title verification: 🏋 has zero uses in 5,877 indexed LibreChat commits through 2026-10-01 21:39:51 UTC; sentinel passed. Frame: bounded CPU work moved off the API thread.