Skip to content

fix(anthropic): bound terminal guard buffering - #85

Closed
luvs01 wants to merge 1 commit into
devfrom
codex/fix-unbounded-streaming-in-terminal-guard
Closed

luvs01 wants to merge 1 commit into
devfrom
codex/fix-unbounded-streaming-in-terminal-guard

Conversation

@luvs01

@luvs01 luvs01 commented Aug 8, 2026

Copy link
Copy Markdown
Owner

Motivation

  • Prevent an unbounded-memory DoS introduced by the Anthropic terminal guard that retained every non-terminal streaming event in-memory until a terminal done was observed.
  • Ensure the guard still detects short no-tool announcements but stops analysing/retaining once a stream is too large or a real tool call appears.

Description

  • Add bounded retention limits: MAX_RETAINED_EVENTS and MAX_RETAINED_CONTENT_CHARS, and track retained text/content size while streaming.
  • Introduce an analysisEnabled flag that is cleared when any bound is exceeded or a tool_call_start is seen, and clear the per-turn seen buffer when analysis is disabled so the guard stops growing memory usage.
  • Only run analyzeTerminalTurn when analysisEnabled is true; otherwise fall back to pass-through behavior and do not attempt continuations.
  • Add a regression test stops retaining events once a streamed answer cannot be a short announcement that verifies oversized/non-terminal streams are still forwarded to the client but not retained for analysis or continuation.

Testing

  • Ran the focused suite bun test tests/terminal-guard.test.ts, which passed (15 tests, 0 failures).
  • Ran type checking with bun run typecheck, which succeeded.
  • Ran the privacy scan with bun run privacy:scan, which succeeded.
  • Ran the full test suite bun run test; a pre-existing unrelated assertion in tests/service.test.ts failed, so the full suite was not green, but the terminal-guard focused tests and relevant regressions all passed.

Summary by CodeRabbit

  • Bug Fixes

    • Improved handling of oversized streamed responses.
    • Streams involving tool calls or excessive content now continue safely without automatic continuation analysis.
    • Prevented unnecessary buffering once response limits are exceeded.
  • Tests

    • Added regression coverage for forwarding oversized streamed answers without triggering continuation.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Aug 8, 2026
@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Changes

Terminal guard bounds

Layer / File(s) Summary
Retention limits and analysis state
src/server/responses/terminal-guard.ts
The terminal guard adds maximum retained-event and retained-content limits. It tracks retention counters and runs terminal analysis only while analysis remains enabled.
Bounded retention and regression coverage
src/server/responses/terminal-guard.ts, tests/terminal-guard.test.ts
Tool calls and retention limits disable analysis and clear buffered events. The regression test verifies that an oversized stream forwards all 2,003 events without auto-continuation.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: ingwannu, lidge-jun, wibias

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: bounding buffering in the Anthropic terminal guard.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-unbounded-streaming-in-terminal-guard

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot changed the title fix(anthropic): bound terminal guard buffering [WRONG BRANCH] fix(anthropic): bound terminal guard buffering Aug 8, 2026
@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown

✅ READY

  • this PR is ready for review.

Hygiene

Deterministic PR hygiene checks passed.

@github-actions
github-actions Bot marked this pull request as draft August 8, 2026 06:36
@luvs01 luvs01 closed this Aug 19, 2026
@luvs01 luvs01 reopened this Aug 19, 2026
@luvs01 luvs01 changed the title [WRONG BRANCH] fix(anthropic): bound terminal guard buffering fix(anthropic): bound terminal guard buffering Sep 3, 2026
@luvs01
luvs01 changed the base branch from main to dev September 3, 2026 06:27
@github-actions
github-actions Bot marked this pull request as ready for review September 3, 2026 06:35
luvs01 added a commit that referenced this pull request Sep 16, 2026
Carry the bounded-retention fix from #85
(source 51723d6) onto current dev.
Preserve upstream passthrough, opt-in, timestamp, and usage behavior;
stop retaining after tool activity, substantive text, or replay limits.
Add focused boundary regressions and document the retention contract.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

luvs01 commented Sep 16, 2026

Copy link
Copy Markdown
Owner Author

상류 dev 기준으로 필요한 수정만 이식한 PR을 게시했습니다: lidge-jun#4739

기존 상류의 heartbeat/tool-delta 통과 처리, 단일 타임스탬프, OpenAI Chat opt-in 동작을 보존하면서 이벤트 수·콘텐츠 보존량 제한과 44개 회귀 검증 사례를 추가했습니다. 게시 커밋: 469e7ab21689d5cf938ea9cf85ef36f9ff977572.

Node 기반 격리 검증은 63 통과 / 0 실패 / 1개 기존 bridge 통합 검증 제외입니다. Bun 전체 검증·타입 검사·CI 통과를 주장하지 않으며, 상류 PR은 해당 검증과 리뷰가 끝나기 전까지 초안 상태입니다. 이 원본 PR은 닫거나 병합하지 않았습니다.

@luvs01

luvs01 commented Sep 16, 2026

Copy link
Copy Markdown
Owner Author

Closing this tracking PR. The terminal guard content retention work was reimplemented against current upstream dev and is being carried forward there, so keeping a second copy of the same change open here only splits review and rebase effort. Nothing in this branch is lost: the implementation, its regression coverage and the documentation all moved with it, and the remaining verification is happening on the branch that is actually up for review.

@luvs01 luvs01 closed this Sep 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aardvark bug Something isn't working codex

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant