🥽 feat: Configure Attached Defaults and Honor Skill Read Ranges - #16540
Conversation
|
Ready for review at exact remote head A1 adds optional attached-environment foreground command and read-window defaults, preserves explicit/background overrides and existing defaults, and bounds timeout guidance by the effective negotiated ceiling. No worker changes. Local verification:
Title verification: 🥽 has 2 indexed commits, both subject-leading, among 5,615 LibreChat commits through 2026-09-28 23:44:55 UTC. The sentinel passed; the checked open-PR snapshot had no matching title. Frame: a configurable but bounded view and execution budget. This PR title records the spend. |
|
Verification update for exact pushed head
No Lighthouse performance result was obtained locally. GitHub CI is running on this head; no failing check was reported in the latest snapshot. No inline review threads or submitted reviews have arrived yet. Not run locally: whole workspace suites, config migration suites, unused-i18n-key sweep, and unused-npm-package sweep. Screenshots were not captured because local Chromium cannot start. No tracked harness changes were made. |
|
Ready for review at exact remote head Finding ledger:
Current local verification:
GitHub CI is running independently on this pushed head. Local Lighthouse will be retried; the previous local attempt could not launch Chromium because the sandbox lacks |
|
Ready for review at exact remote head Finding ledger:
After two actionable rounds, the parent swept the subsystem by authorization, identity, schema ownership, default/ceiling separation, state/replay, cancellation, reads, and mixed-version behavior. The full production diff and affected SDK boundaries/callers were inspected. A fresh independent review is being requested on this head. Local checks:
GitHub CI runs on this head. Local Lighthouse will be retried; earlier local attempts were blocked by missing system |
|
Independent PR Reviewer completed review of exact head Result: Complete, no new findings. Both ledger findings were confirmed fixed:
No rejected or open findings. After two actionable rounds, the parent completed a subsystem invariant sweep. The final independent reviewer read the full frozen diff and affected paths and ran a dependency-free frozen-source harness. Repository Jest and real LangChain invocation were not rerun in the review environment; the parent's real-SDK regressions and focused repository tests passed before publication. No live-worker, actual proxy-budget, or mixed-replica rollout testing was performed. Current-head CI is green, including Lighthouse, API/data-provider tests, type checks, static checks, and integrations. The latest authoritative check snapshot has no pending or failing checks. Workflow-gated e2e, MCP list_changed, Bombadil, and one Codegraph-select job were skipped. No inline GitHub review threads remain; no submitted GitHub review is claimed by this independent-review comment. Parent local verification: 770 packages/api tests, 143 production loader tests, and 289 configuration tests passed (1,202 total). Both changed TypeScript workspaces passed Not run locally: whole workspace suites, config migration tests, unused-i18n and unused-package sweeps, live-worker/proxy-budget/mixed-replica rollout tests, and browser screenshots. |
|
Ready for review at exact remote head This head adds the requested read-range correction to A1:
The regression run on the earlier head confirmed explicit ranges returned the complete file. Current source-path tests pass; final focused tests/typechecks/build/static checks run in parallel with this head's CI. A fresh independent PR Reviewer is being requested for this head. Prior clean review covers Acceptance measurement: a synthetic 660-line catalogue reproduction through the real handler returned exactly 60 and 85 lines. With o200k_base, the full output was 4,742 tokens; those windows were 463 and 638 tokens respectively. These are fixture measurements, not verification of the reported 19 historical reads or their aggregate savings. Historical traces were not supplied. Existing ledger: R1-P2-1 fixed in Title refresh retains this PR's already-used 🥽, connecting to a bounded view. Verified: 2 prior subject-leading uses among 5,639 commits through 2026-09-30 09:10:10 UTC; sentinel 🧹=54 passed. The applied PR title is the spend record. |
|
Verification for exact pushed head
Total focused repository tests: 1,273. The separate disposable output-measurement harness also passed. The real read handler was exercised with a synthetic 660-line catalogue reproduction and
The token reductions are 90.2% and 86.5% for this synthetic fixture. This is not a replay of the actual SQL catalogue or the 19 historical reads: the original traces were not supplied and the skill source repository was inaccessible to this environment. No historical aggregate token-savings claim is made. A fresh independent review is running on this head. The earlier complete review of Not run locally: whole workspace suites, config migration tests, unused-i18n and unused-package sweeps, live-worker/proxy-budget/mixed-replica testing, browser screenshots, or live deployed skill-library invocation. Local Lighthouse: Current-head CI snapshot: 11 pending; 0 failing. All claimed CI results are scoped to this head. |
|
Independent PR Reviewer completed review of exact pushed head Result: Complete, no findings. The review covered the full 17-file diff and the four-file skill/sandbox range extension. Both earlier P2 findings remain resolved:
No open or rejected findings. The reviewer traced consumers, full-content filters, storage/cache writes, schemas, byte truncation, continuation, and binary/image paths. Dependency-free frozen-source probes covered all four local text sources, validation, filtering before slicing, complete streamed cache writes, EOF/blank lines, UTF-8 truncation, and already-paged workspace output. Review limitations: repository Jest, TypeScript, and real SDK integration were not rerun in the isolated reviewer environment because dependencies were unavailable. The parent ran the focused repository suites, real-SDK regressions, both changed workspace typechecks, builds, and static checks. No live-worker execution was performed. Latest exact-head CI: no failures. Tests, typechecks, static checks, integrations, builds, and Lighthouse passed. Local verification: 841 packages/api tests, 143 loader tests, and 289 configuration tests passed (1,273 total). Both changed workspaces passed The synthetic 660-line catalogue reproduction returned exactly 60/85 requested lines and measured 463/638 o200k_base tokens versus 4,742 for the full read. These are fixture measurements, not a replay of the actual codegraph catalogue or the 19 historical traces. Not run locally: whole workspace suites, config migration tests, unused-i18n/package sweeps, live-worker/proxy-budget/mixed-replica testing, browser screenshots, or deployed skill-library invocation. |
|
Ready for review at exact rebased head Rebased the four scoped commits onto A1 still carries configurable attached foreground command/read defaults and explicit skill/sandbox read ranges with full-file filtering, unchanged no-range reads, and shared continuation formatting. The two prior P2 ledger findings remain fixed in the rebased history. Previous reviews and CI cover earlier SHAs, not this head. Dependencies are being refreshed for dev's agents SDK upgrade. Focused repository tests, both changed-workspace typechecks, builds, static checks, and a fresh independent review are running alongside GitHub CI. No current-head passing checks or clean review are claimed yet. |
c30de95 to
aa5e48f
Compare
|
Ready for review at exact rebased head The initial rebased head The corrected code passed focused tests, package builds, and scoped static checks with SDK 4.0.1. The interrupted dependency refresh left missing declaration files locally, so a clean lockfile install was completed and both workspace typechecks and focused checks are being rerun on this exact head. Do not treat any earlier-head results as covering this head. The rebased ledger is R1-P2-1 fixed in |
|
Verification for exact rebased head
Total focused tests: 1,358. Dependencies were refreshed from the rebased lockfile; agents SDK 4.0.1 is installed. The first dependency refresh was interrupted and left missing declaration files; those results were discarded and a clean lockfile install plus all checks were rerun. Rebase conflict resolution was import-only. Initial rebased head Ledger: R1-P2-1 remains fixed in rebased Not run locally: whole workspace suites, config migration tests, unused-i18n/package sweeps, live-worker/proxy-budget/mixed-replica rollout, historical trace replay, or screenshots. Current-head CI snapshot: 11 passing checks, 29 pending, 0 failing. Local |
|
Ready for review at exact pushed head R4-P2-1 (P2), "Retrieve complete sandbox text before slicing line ranges": fixed in Explicit sandbox ranges now use the existing windowed byte transport and require complete retrieval within the existing 256 KiB byte budget. Full text is validated, filtered, then sliced. Incomplete or oversized retrieval cannot produce a false EOF. Legacy no-range reads, images, and worker contracts are unchanged. A small TS adapter owns completeness; CJS changes are export wiring only. Local checks: 909 packages/api tests, 378 callback/loader/transport tests, and 346 config tests passed (1,633 total). Both workspace typechecks, real API build, scoped static checks and whitespace checks passed. Tests include the production adapter and handler under capped responses, protected content beyond the stdout prefix, adaptive smaller-cap windows, complete UTF-8 assembly, route/session/auth preservation, and cancellation. Prior findings R1-P2-1, R2-P2-1, and RB-P1-1 remain fixed. No rejected findings. A fresh independent review and GitHub CI run on this exact head; the previous review covers Not run locally: whole suites, config migration tests, unused-i18n/package sweeps, live-worker/proxy-budget/mixed-replica tests, historical trace replay, or screenshots. |
|
Independent PR Reviewer completed review of exact pushed head Result: Complete, no findings. All four ledger dispositions were independently rechecked:
No open or rejected findings. The reviewer inspected the full frozen diff, SDK 4.0.1 archive, and production callback harnesses under simulated 64 KiB/16 KiB stdout caps. Completeness, byte limits, malformed output, invalid UTF-8, cancellation, execution identity, full-text filtering order, pagination, and timeout contracts were checked. Harnesses mocked transport and external dependencies; repository Jest, TypeScript, full LangChain invocation, and live-worker checks were not rerun in that environment. Parent local verification: 1,633 focused tests passed (909 packages/api, 378 callback/loader/transport, 346 config), both changed workspace typechecks passed, real API build and committed-diff static checks passed. Local Lighthouse was attempted but blocked by missing Exact-head CI has no failures. Lighthouse, typechecks, backend/frontend tests, static checks, builds, integrations, production runtime smoke, MCP Apps/list_changed, and Redis-transport e2e passed. Six memory-e2e shards remain running. Bombadil and one workflow-gated Codegraph-select check were skipped. No GitHub inline review threads exist; this comment records the independent review, not a submitted GitHub approval. Not run locally: full workspace suites, config migrations, unused-i18n/package sweeps, live-worker/proxy-budget/mixed-replica rollout tests, historical trace replay, or screenshots. PR remains open and unmerged. |
Summary
Attached Bash always uses a 30-second foreground timeout when the model omits
timeoutMs, even when the environment supports longer commands. Attachedread_filesimilarly requests 200 lines whenmax_linesis omitted. Operators can now configure those defaults per environment throughconfigSchema.limits.defaultCommandTimeoutMsanddefaultReadFileLines.Omission preserves the existing defaults. A foreground timeout never raises the administrator, worker, protocol, or verified HTTP-budget ceiling. Explicit timeouts still win, and detached background calls still default to the effective maximum. Programmatic tools retain the SDK's attached-workspace instructions, input-file/artifact locations, intent and tool-manifest fields, and validation bounds; only omission-time timeout defaults and guidance are customized. Timed-out command results retain stdout, stderr, termination markers, and truncation markers, then explain the actual timeout, effective retry ceiling, optional background execution, and possible partial side effects.
Workspace read windows can be configured up to the existing 500-line ceiling. Skill
SKILL.mdbodies, cached bundled text, streamed bundled text, and sandbox text now also honor explicitstart_lineandmax_linesvalues instead of returning the whole file. A request for lines 100–159 returns only those 60 lines, numbered from 100, with a continuation pointing to 160. No range still returns the existing full skill/sandbox read, including unchanged size-limit and binary/image behavior. Full-content filtering happens before slicing, and streamed caches retain the complete text. Explicit sandbox ranges first retrieve the complete file within the existing 256 KiB budget through the already-windowed byte transport; truncated stdout is never treated as EOF. Oversized or incomplete retrieval fails without exposing a partial page. Workspace responses share the validator and formatter but are not sliced a second time. This PR does not change worker contracts, shell state,cdhandling, search, or patch/edit tools.How it works
Example settings beneath an attached environment:
HTTP admission, settlement, and delivery reserves can lower the effective maximum. Update all LibreChat API replicas before enabling new strict configuration fields. No worker update or data migration is required.
Type of change
Testing
Focused coverage includes configured and omitted defaults, explicit and background overrides, administrator/worker/HTTP ceilings, invalid configuration, timeout diagnostics, wider reads, pagination, shared-schema isolation, code-to-skill upgrades, skill-first registration, production tool-loader wiring, all three skill-text sources, sandbox ranges, full-file filtering outside the requested window, EOF/blank lines, UTF-8 byte truncation, complete cache writes, images/binary metadata, and SDK schema-field preservation.
Local checks and GitHub CI results are recorded in the head handoff comment.
Screenshots / recordings
No UI component, layout, or control changes. Model-facing tool descriptions, line-window results, continuation text, and timeout results are covered by focused execution tests. No UI screenshots captured.
Risk / compatibility
timeoutis omitted. Explicit requests retain the independent negotiated ceiling. Without a configured foreground default, the legacymaxCommandTimeoutMsbehavior is preserved. The SDK has a 1,000 ms minimum; with the new default configured, a ceiling below that minimum refuses programmatic execution rather than increasing its budget. Usebash_toolfor shorter budgets.Checklist
librechat.example.yaml; read-range guidance ships in the tool schemas