Skip to content

Guard app-server against oversized rollout hydration - #1

Draft
offx366 wants to merge 1 commit into
mainfrom
fix/bound-legacy-rollout-hydration
Draft

offx366 wants to merge 1 commit into
mainfrom
fix/bound-legacy-rollout-hydration

Conversation

@offx366

@offx366 offx366 commented Aug 31, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • add an opt-in byte budget to full rollout hydration
  • apply a 100 MiB fail-closed limit to legacy thread history reads
  • preserve metadata-only thread reads and the existing unbounded API for callers that explicitly need it
  • cover exact-boundary and oversized behavior for plain and compressed rollouts

Why

RolloutRecorder::load_rollout_items currently parses every JSONL record and retains every decoded RolloutItem in one Vec. A single oversized rollout can therefore make thread/read or resume consume many times the file size in app-server memory.

This change puts the guard at the legacy thread-store full-history boundary. Files over 100 MiB now return a clear InvalidRequest before ordinary JSONL parsing starts. The source rollout is left byte-for-byte untouched, and metadata-only listing/read paths continue to work.

The 100 MiB threshold is intentionally conservative: issue openai#29510 includes a report where a 106 MB rollout was already enough to stall a 16 GB machine.

This addresses the unbounded full-history hydration path reported in openai#29510. It does not attempt automatic truncation, quarantine, or migration; those would need separate product decisions.

Validation

  • cargo +1.95.0 fmt --all -- --check
  • git diff --check
  • added unit coverage for plain-file metadata rejection, exact-boundary success, compressed decoded-size rejection, and thread-store error mapping

Targeted Rust execution is left to CI because compiling the workspace locally exceeded the constrained test host's safe disk budget; the PR is draft until those checks pass.

Fork CI note: a manual rust-ci dispatch was attempted, but GitHub rejected the jobs before allocating a runner because Actions are locked for the fork owner's account. No test or build step ran, so that infrastructure failure is not a code result.

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.

1 participant