Skip to content

fix: bound incremental cost session events - #151

Closed
GautamSharma99 wants to merge 1 commit into
openai:mainfrom
GautamSharma99:fix/bound-cost-session-events
Closed

fix: bound incremental cost session events#151
GautamSharma99 wants to merge 1 commit into
openai:mainfrom
GautamSharma99:fix/bound-cost-session-events

Conversation

@GautamSharma99

Copy link
Copy Markdown
Contributor

Summary

Bound the incremental Codex session JSONL reader so a malformed or unexpectedly large unterminated event cannot consume unbounded memory or trigger quadratic buffer copying during cost polling.

Fixes #138.

What changed

Bounded event parsing

Cost-session JSONL events are now limited to 1 MiB. The reader enforces the limit while each 64 KiB file chunk is processed, so an oversized event is rejected before it can continue accumulating in memory.

The error is intentionally fixed and does not include any session content:

Codex session event exceeds the 1 MiB safety limit.

This error flows through the tracker's existing onError path, which lets a budgeted scan abort when its running cost can no longer be verified safely.

Linear incremental buffering

The previous implementation concatenated the complete partial line with every new read. For an unterminated event, that repeatedly copied all bytes seen so far and made parsing approximately quadratic.

The new parser stores bounded fragments and tracks their aggregate byte count. Each fragment is copied once while the event is incomplete, and the fragments are concatenated only once when a newline completes the event. Events contained within a single read are parsed directly without an intermediate concatenation.

This preserves normal JSONL append behavior, including events split across multiple reads and multiple polling cycles.

Session quarantine

When a session exceeds the event limit, it is marked unreadable and its retained fragments are released. Later polling cycles skip that session instead of repeatedly revisiting the same invalid stream or retaining its partial contents.

Already parsed metadata and token usage remain available in the current snapshot, but the overflow is still surfaced immediately through the tracker error path.

Documentation

The TypeScript README now documents the 1 MiB cost-session event limit and explains that an oversized event stops a budgeted scan because live cost can no longer be verified safely.

Why this matters

A corrupt session file or a very large partial write can no longer:

  • grow the tracker process's memory without a ceiling;
  • repeatedly recopy an ever-growing partial event;
  • delay cost-budget enforcement while the parser processes unbounded input;
  • expose raw session content in the resulting diagnostic.

Tests

Added regression coverage for:

  • a valid event split across reads and polling cycles;
  • a partial event that spans multiple 64 KiB reads while remaining below the limit;
  • an event exceeding 1 MiB without a newline;
  • release and quarantine of oversized session state;
  • continued access to the last valid cost snapshot after quarantine.

Verification performed:

  • pnpm run types
  • pnpm run format
  • pnpm run build
  • focused cost-tracker tests
  • pnpm run test

Full suite result:

  • 472 passed
  • 6 expected platform/integration skips
  • 0 failed

@rajpratham1 rajpratham1 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.

This pull request strengthens the session cost tracker by replacing the unbounded remainder buffer with bounded incremental line buffering and enforcing a 1 MiB maximum session event size. The new implementation preserves partial events across multiple reads while preventing malformed or excessively large events from consuming unbounded memory. When the limit is exceeded, the session is quarantined so subsequent reads do not continue processing an unreadable stream. The accompanying tests verify both successful reconstruction of fragmented events and correct handling of oversized events, and the README documents the new behavior. Based on the visible changes, the implementation is focused, aligns with its stated purpose, and no blocking issues are apparent.

@mldangelo-oai

Copy link
Copy Markdown
Collaborator

Thanks for tracking down the incremental cost-event buffering issue. The bounds proposed here are already present on main, so this PR no longer adds a distinct change. I'll close it as superseded. We'd welcome another contribution if you find a case the current cost tracking still misses.

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.

Cost tracker keeps an unbounded partial JSONL event and repeatedly recopies it

3 participants