Repository navigation
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Repository: luvs01/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced 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 |
|
✅ Deterministic PR hygiene checks passed. |
✅ READY
Hygiene✅ Deterministic PR hygiene checks passed. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 327635678e
ℹ️ 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".
| if (guardHeldBytes <= CURSOR_OUTPUT_GUARD_MAX_HOLD_BYTES) return true; | ||
| echoSniffer?.finish(); | ||
| routingCommentarySniffer?.finish(); | ||
| releaseGuardHeld(); |
There was a problem hiding this comment.
Inspect cap-crossing text before disarming the quarantine
When retained reasoning leaves guardHeldBytes just below 8 KiB and the next first text_delta both crosses the cap and begins with [Tool Result] or matching routing-failure commentary, this branch settles both sniffers and emits the held events before the caller feeds that text to them. The invalid marker therefore reaches the client and sets emittedOutput, preventing the corrective retry even though nothing had escaped before this delta; evaluate a cap-crossing text event with the active sniffers before deciding to flush and disarm.
AGENTS.md reference: src/AGENTS.md:L17-L19
Useful? React with 👍 / 👎.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> # Conflicts: # src/adapters/cursor.ts # tests/cursor-envelope-echo-retry.test.ts
Two Devin Review findings on the turn-start output quarantine: - A single oversized event (Cursor accepts frames up to 16 MiB) was fully retained and re-encoded before the 8 KiB cap released it. The retained size is now projected from payload length before any serialized copy exists; a frame that cannot fit settles the sniffers, releases the held events, and passes through directly. - The aggregate cap was checked before the armed sniffers saw the text, so a >8 KiB first delta could bypass echo/routing classification entirely. Text deltas are now classified first (on the bounded leading window each sniffer needs) and only then subject to the cap. Adds regression tests for oversized single deltas carrying each guarded pattern and one oversized reasoning frame, and documents the aggregate limit/disarm contract in structure/providers/cursor.md. Co-Authored-By: Epinephrine <luvs01@hanmail.net>
|
Closing: superseded by combined PR #610, which carries this PR's bounded output-guard reasoning quarantine together with the echo-diagnostics retention change on current dev. Combined tests: 47 + 54 pass. |
|
Fixed at head (1aae049): text deltas are now classified by the armed echo/routing sniffers on each sniffer's bounded leading window BEFORE the aggregate-cap check, so an oversized first delta can no longer bypass detection. Regression tests cover oversized single deltas carrying each guarded pattern in tests/providers/cursor/cursor-envelope-echo-retry.test.ts. |
|
Fixed at head (1aae049): retained size is now projected from payload length plus fixed per-event overhead BEFORE any encoded copy exists; an event that cannot fit the remaining 8 KiB budget settles both sniffers, releases the held events, and is emitted directly without being serialized. A regression covers one reasoning frame larger than the cap. |
|
Done: structure/providers/cursor.md documents the aggregate limit and disarm behavior (1aae049), and c1506e3 now records the same retained-quarantine bound under Stream-buffer accounting in structure/transports/byte-accounting.md — the contract authority for adapter byte bounds (src/adapters/ is one of its covered areas). |
Motivation
thinking_deltaevents so an attacker or compromised provider cannot exhaust process memory during tool-result continuations.Description
CURSOR_OUTPUT_GUARD_MAX_HOLD_BYTESand reuse it in the envelope sniffers (src/adapters/cursor/envelope-echo.ts).createCursorAdapterby addingguardHeldBytes,guardEncoder, andholdGuardEvent(event)so reasoning frames contribute to the same 8 KiB cap and trigger a flush/disarm when exceeded (src/adapters/cursor.ts).echoSniffer?.finish()/routingCommentarySniffer?.finish()andreleaseGuardHeld()to flush and disarm the quarantine so held events are emitted instead of growing unbounded.tests/cursor-envelope-echo-retry.test.ts).Testing
bun test tests/cursor-envelope-echo-retry.test.tsand they passed (all tests in that file passed).bun run typecheckandbun run privacy:scan, both succeeded.bun run test); repository-wide integration/Lab tests showed unrelated environment-dependent failures (sandbox proxy env checks, some long integration timeouts and CLI help differences) and therefore are outside the scope of this focused fix.Codex Task