Skip to content

🪟 feat: Preserve Command Output Heads and Tails - #275

Open
lia-by-librechat[bot] wants to merge 1 commit into
mainfrom
lia/command-output
Open

lia-by-librechat[bot] wants to merge 1 commit into
mainfrom
lia/command-output

Conversation

@lia-by-librechat

Copy link
Copy Markdown
Contributor

Summary

Attached native commands currently keep only the first output bytes that reach a shared stdout/stderr budget. Long test runs lose their final summaries, and noisy stdout can consume the entire allowance before an error reaches stderr.

Keep a bounded prefix and rolling suffix for each stream, then fit both into the existing combined maxOutputBytes limit. Inline [... N bytes omitted ...] markers report dropped raw bytes. Stdout and stderr remain separate; quiet streams donate unused space, while noisy streams split the budget with the odd byte reserved for stderr. Sandbox violation annotations enter stderr before final rendering.

This is B1 only. LibreChat A1/A2 remain in #16539 and #16540. No parent creation, read/search changes, edits, model-family selection, or patch execution is included. This does not depend on worker #271.

Mechanism

child stdout / stderr
  → per-stream copied prefix + fixed-capacity suffix ring
  → append sandbox diagnostics to stderr
  → balance stream budgets and render UTF-8-safe windows + omission markers
  → unchanged execute_command result and existing validators
  • Retained raw output is at most twice maxOutputBytes, independent of total output and chunk count. No references to oversized child buffers or per-chunk buffer arrays survive capture.
  • UTF-8 boundaries, malformed-byte replacement expansion, and omission markers fit the combined response byte budget.
  • Tiny stream allowances that cannot fit an omission marker still return the existing truncated flag.
  • Small output remains unchanged. Truncation never stops execution. Timeout/signal metadata, cancellation errors, mutation certainty, and cleanup remain unchanged.
  • No new protocol, capability, configuration, or response keys. Existing Code API and LibreChat readers accept the same result shape; no reader-first rollout is required.

Verification

Focused checks and exact-head review results are recorded in the head handoff comment.

  • Collector tests cover chunk-independent suffix retention, exact omission counts, stream fairness, quiet-stream donation, tiny limits, UTF-8 and invalid bytes, exact-fit output, and copying rather than retaining source chunks.
  • Native command regressions use real Bash for both-stream summaries, sandbox annotations, and timeout settlement, and validate the legacy result shape.
  • Local collector-boundary probes run real Bash directly through the production spawned-process collector. Summary and timeout probes fail on the previous collector and pass with B1; cancellation semantics pass on both.
  • The local public native-sandbox.test.ts suite is blocked before command execution by the existing private-storage ancestor guard because this worker sandbox's / belongs to uid 65534. Unchanged main reproduces that failure. The guard was not weakened. Collector-boundary probes do not certify SRT admission or confinement.

Not run locally: the complete packages/code suite, live SRT confinement tests, service/API builds or Bun suites. Only packages/code source changes. No deployment or benchmark improvement is claimed.

@lia-by-librechat

Copy link
Copy Markdown
Contributor Author

B1 head handoff

Pushed head: f11f51d036d04199f9036531003822f2b7367b6e
Base and merge-base: 3189cfd8ee7f81cf90671f9b91b1d52ae9d3693d

Exact-head local check Result
packages/code: tsc --noEmit Passed
packages/code: real npm run build Passed
output.test, native-process.test, protocol.test 55 passed, 0 failed
Real Bash through the production spawned-process collector 3 passed: both-stream summaries/annotations/legacy validator, timeout settlement, typed cancellation
New collector/test Prettier check and git diff --check Passed

The summary and timeout probes fail on the previous collector; cancellation remains unchanged. The local public native sandbox suite stops at the existing private-storage ownership guard before command execution, and unchanged main reproduces that root-ownership rejection. CI's three full code-package matrix lanes and Linux native-sandbox lane passed this exact head.

Existing native-sandbox.ts, native-sandbox.test.ts, and the README already fail full-file Prettier on the base revision. Newly added test blocks were formatted and unrelated formatting was preserved. There is no packages/code ESLint/import-sort task. The complete local code-package suite, live local SRT confinement tests, and local service/API builds or Bun suites were not run.

Independent review was requested for this exact head. The child runner failed to complete and returned no review verdict or finding ledger. This is not a clean review; severity counts and dispositions are unavailable. No GitHub inline review findings were present at the handoff check.

A1/A2 PRs are untouched. No deployment, merge, or benchmark improvement is claimed.

@LibreChat-AI LibreChat-AI deleted a comment from lia-by-librechat Bot Sep 30, 2026
@danny-avila

Copy link
Copy Markdown
Collaborator

@codex review

Please review B1 at exact pushed head f11f51d036d04199f9036531003822f2b7367b6e against merge-base 3189cfd8ee7f81cf90671f9b91b1d52ae9d3693d.

Scope: bounded head-and-tail native command output, separate stdout/stderr, stderr reservation, inline omission counts, UTF-8/combined-budget enforcement, and unchanged timeout/cancellation/result contracts. A1/A2 and filesystem/search/patch slices are excluded.

Local focused collector/protocol/subprocess checks and real-Bash collector probes pass. Public native sandbox tests are blocked before execution by the existing private-storage ancestor guard in this worker; unchanged main reproduces the same root-ownership rejection. Full CI and independent exact-head review run separately.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-30T10:39:07.476427Z f11f51d Manual request
ℹ️ 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" or "@codex security review".

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 chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f11f51d036

ℹ️ 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 +176 to +179
const stdoutBudget = Math.min(
stdout.bytes,
budget - stderrReserve + Math.max(0, stderrReserve - stderr.bytes)
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Allocate quiet-stream space using rendered byte sizes

When one stream is noisy and the other contains malformed UTF-8, this calculation donates space based on the quiet stream's raw byte count even though decoding may expand each invalid byte into a three-byte replacement character. For example, with a 32-byte limit, large stdout, and stderr containing two 0xff bytes, stdout receives 30 bytes and stderr.render(2) returns an empty string, even though the six rendered stderr bytes could fit by reducing stdout's allocation. This can completely starve stderr in exactly the malformed-output case the collector supports; budget using the rendered size or rebalance unused space after rendering.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants