Skip to content

fix: normalize locked rate-limit reader acquisition - #687

Merged
seonghobae merged 3 commits into
mainfrom
fix/rate-limit-reader-acquisition-20260913
Sep 12, 2026
Merged

fix: normalize locked rate-limit reader acquisition#687
seonghobae merged 3 commits into
mainfrom
fix/rate-limit-reader-acquisition-20260913

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Finding

The protected distributed rate-limit transport already bounds the private request to 256 bytes and the private decision to 4,096 bytes, releases acquired reader locks, and treats cancellation as best-effort cleanup. Reader acquisition itself was still outside that normalization boundary: a synchronous getReader() failure on a locked/non-stream-readable internal request escaped the established 400 malformed_json contract before storage authority, while the decision path was wrapped only later with raw engine exception text rather than the stable rate-limit Durable Object decision body could not be read diagnostic.

Test-first RED

Test-only exact e755e86908bbd3cf7cb838380cb1f85cf3a8db21 adds hostile synchronous reader-acquisition failures for both private paths. Hosted application CI 34711502421 reached exact checkout, toolchain, live-base, lockfile, install, and release typecheck successfully, then failed at release tests as intended.

Minimal causal repair

Final exact 98ec07552f4081bd4ff6b25f1f3ed691c0f3f2b9 normalizes only reader acquisition: the request path returns the pre-existing 400 malformed_json result before storage authority, and the decision path raises the pre-existing stable DistributedRateLimitUnavailable("rate-limit Durable Object decision body could not be read") contract. Fixed byte ceilings/buffers, cancellation-liveness, post-acquisition lock release, fatal UTF-8/duplicate-key/JSON admission, bucket/state semantics, and endpoint/media-type behavior are unchanged. An intermediate wording-only drift in an unrelated docstring was removed before the final exact head.

Exact-head gates are GREEN: application CI 34711784552, reviewer-ci 34711784561, Security Scan 34711784541, and patch-validator-image 34711784554 all completed successfully. Reviewer CI retained the 100% line+branch and 100% docstring gates for Noema core and reviewer and passed the real no-network CodeGraph sandbox.

No provider/model routing, outbound authority, credential authority, quarantine/security authority, release/deployment authority, or foreign domain truth is transferred by this change.

Summary by CodeRabbit

  • 버그 수정

    • 읽을 수 없는 요청 본문을 처리할 때 더욱 명확한 400 malformed_json 오류를 반환합니다.
    • 속도 제한 서비스의 응답 본문을 읽을 수 없는 경우 일관된 서비스 이용 불가 오류로 처리합니다.
    • 요청 및 응답 본문이 잠긴 상태에서도 크기 제한, 취소, 안전 차단 동작이 유지됩니다.
  • 테스트

    • 읽을 수 없는 요청 및 응답 본문에 대한 오류 처리 시나리오를 추가했습니다.

@coderabbitai

coderabbitai Bot commented Sep 12, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 5f753cd2-a2e7-4756-adb2-49cd6a92c7b1

📥 Commits

Reviewing files that changed from the base of the PR and between 38fc806 and 98ec075.

📒 Files selected for processing (2)
  • src/rate-limit.ts
  • test/rate-limit-locked-body.test.ts

📝 Walkthrough

Walkthrough

요청 및 Durable Object 결정 응답의 getReader() 실패를 명시적으로 처리합니다. 요청 실패는 400 malformed_json으로 반환합니다. 응답 실패는 DistributedRateLimitUnavailable로 변환합니다. 두 동작을 테스트합니다.

Changes

Rate limit reader 오류 처리

Layer / File(s) Summary
Reader 획득 실패 정규화 및 검증
src/rate-limit.ts, test/rate-limit-locked-body.test.ts
요청 본문 reader 획득 실패를 400 malformed_json 응답으로 변환합니다. Durable Object 결정 응답 reader 획득 실패를 DistributedRateLimitUnavailable 예외로 변환합니다. 두 경로에서 저장소 트랜잭션과 오류 메시지를 검증합니다.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

✨ 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 fix/rate-limit-reader-acquisition-20260913

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.

@seonghobae
seonghobae marked this pull request as ready for review September 12, 2026 18:41
@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.

@seonghobae seonghobae left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exact-head review: no valid finding remains in the two-file diff. The hostile RED covers synchronous reader acquisition failure on both private rate-limit paths; the repair only normalizes acquisition into the existing request/decision contracts. Fixed byte ceilings and retained buffers, cancellation cleanup, post-acquisition lock release, strict UTF-8/JSON admission, Durable Object bucket semantics, and Noema/foreign-owner authority boundaries remain unchanged. Exact-head CI/reviewer/security/image gates are GREEN; this is a COMMENT review, not self-approval.

@seonghobae
seonghobae merged commit f9d08f4 into main Sep 12, 2026
17 of 18 checks passed
@seonghobae
seonghobae deleted the fix/rate-limit-reader-acquisition-20260913 branch September 12, 2026 18:41
seonghobae added a commit that referenced this pull request Sep 12, 2026
seonghobae added a commit that referenced this pull request Sep 12, 2026
seonghobae added a commit that referenced this pull request Sep 12, 2026
seonghobae added a commit that referenced this pull request Sep 12, 2026
…rity-20260913

docs: converge #687 rate-limit reader authority
seonghobae added a commit that referenced this pull request Sep 13, 2026
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