[WRONG BRANCH] claude: avoid retaining streamed reasoning fallback to prevent aggregate OOM - #450
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Team Run ID: 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. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
✅ Deterministic PR hygiene checks passed. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71fb4c6678
ℹ️ 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".
| const signature = open.reasoningSig ?? encodeReasoningEnvelope({ txt: open.thinkingBuf ?? "" }); | ||
| // Streamed thinking has already been delivered to the client. Keep the fallback | ||
| // signature bounded instead of retaining and re-encoding the entire untrusted block. | ||
| const signature = open.reasoningSig ?? encodeReasoningEnvelope({ txt: "" }); |
There was a problem hiding this comment.
When a streamed reasoning block has no genuine upstream signature and Claude Code later replays that assistant block with a tool result, src/claude/inbound.ts preserves the visible thinking in summary alongside this envelope, but src/responses/parser.ts:284 evaluates envelope?.txt ?? text; because the envelope explicitly contains txt: "", it selects the empty value and then drops the reasoning item at line 300. This removes the reasoning_content preceding the replayed function call, so strict providers such as DeepSeek can reject the continuation instead of completing the tool loop. Keep the fallback bounded without setting an overriding empty txt, or make the parser fall back to the visible summary when this marker is empty.
AGENTS.md reference: src/AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
⏳ DRAFT
What to do
Its title has been prefixed with |
github-actions
Bot
changed the title
1 participant
Motivation
response.reasoning_*_text.deltainto a per-turnthinkingBufand encoded the whole retained text on block closure, allowing a malicious upstream to inflate process memory across concurrent turns and cause an aggregate denial-of-service.Description
thinkingBuf/thinkingBufBytesretain/release path and the replacement of transient reasoning deltas with a large fallback envelope. Streamed deltas are still forwarded asthinking_deltaevents.encodeReasoningEnvelope({ txt: "" })instead of re-encoding the entire retained text.tests/claude-integration/claude-outbound.test.tsto assert that streamed reasoning does not create retainedreasoningbudget allocations and that fallback signatures remain bounded and empty when appropriate.Testing
bun test tests/claude-integration/claude-outbound.test.tsand it passed (50 tests) validating streaming behavior, part separators, and bounded fallback signatures.bun run typecheckandbun run privacy:scan, both of which passed.git diff --check(no whitespace errors) and local test iterations; the repository-widebun run testwas executed but reported a large set of unrelated/environment-sensitive failures while the focused change and its targeted tests passed.Codex Task