Conversation
Carry only the Claude skill marker paths from the source change. Read at most 4097 path characters before deciding whether the marker can be elided, and keep oversized marker paths unchanged. Preserve Windows/POSIX basename and the default claude-api blocklist and explicit blockedSkills override behavior. Partial source paths: src/claude/inbound.ts; tests/claude-integration/claude-inbound.test.ts. The source commit also contains separate Kiro and Fernet changes, not included here. (cherry picked in part from commit fe13531)
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: lidge-jun/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe change bounds Claude bundled-skill directory parsing to 4,096 UTF-16 code units. Longer or unterminated directory lines remain unchanged. Tests and documentation cover the new boundary and preserved behavior. ChangesClaude skill-text elision
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to No merge-blocking risk was identified in the bounded Claude skill-text parsing change. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
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. |
리뷰 · 우선순위 44 / 80이 PR은 Claude Code가 보낸 “스킬 문서 묶음” 텍스트를 줄일 때, 첫 줄에 있는 경로를 끝없이 읽지 않도록 막아 줍니다. 라우팅된(Anthropic이 아닌) 모델로 갈 때 라인 - 라인 - 라인 - 끝 슬래시 제거 후 라인 - #5310 — 같은 “skill-path line slice”가 아직 열린 큰 PR에 남아 있습니다. 이 PR이 머지되면 #5310의 Claude 부분은 중복이 됩니다. Fernet·나머지는 이 PR 범위 밖입니다. 라인 - Draft + 보안 체크 미체크 + 포커스 테스트만 — 호스트 풀 CI·exact-head는 작성자도 주장하지 않았습니다. types/config 분할·프리뷰 배포와는 무관합니다. 메인테이너의 판단이 필요한 지점 4,096 넘는 경로 줄을 “통과”로 둘지, 창 안의 마지막 구간만으로라도 basename을 짐작해 막을지. #5310은 이 PR·#5553·Fernet 조각이 다 올라온 뒤 닫거나 Claude diff만 빼서 좁힐지. Draft를 Ready로 올리기 전에 호스트 CI(관련 샤드) 초록을 머지 조건으로 둘지. 너의 추천 방향이 맞고, 한도·의도적 통과·테스트 경계가 한줄로 읽힙니다. Ready로 바꾼 뒤 관련 CI만 확인하면 머지해도 됩니다. 긴 경로 통과는 문서화한 DoS 방어 선택이니 기본은 유지하고, 우회가 걱정되면 후속으로 “창 끝 근처 basename”을 따로 논의하세요. 머지 후에는 #5310에서 Claude 경로 조각을 빼거나, 나머지가 다 쪼개졌으면 #5310 닫기를 권합니다. 지금 닫을 types/config 중복 PR은 없습니다. 이 댓글은 grok-bot이 작성했습니다 |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Summary
Bound Claude bundled-skill marker inspection to a 4,096-code-unit directory line, with one extra character to detect the terminating newline. Oversized or newline-free directory lines remain unchanged; normal POSIX, Windows, mixed and UNC basename matching is preserved. The default
claude-apiblocklist, explicit empty-list opt-out, 10,000-character payload threshold and native Anthropic passthrough are unchanged.This carries only the Claude source/test portion of #5310, retaining its source author/date and commit attribution. Kiro is consolidated in #5553, while the Fernet portion remains separate; this PR alone does not fully supersede #5310.
Verification
305d53910edd276e080219ace13e7653fc620513, based on deva077087b741a7f7cb1d07b801ec831965d180839.tests/claude-integration/claude-inbound.test.tspassed 52 tests / 0 failures, including default/disabled/custom policy, Windows paths and exact 4,096/4,097 boundaries. The newline-free fixture was then strengthened to end in a matching basename after 10,000 path segments; that changed case passed separately (1 test / 0 failures).The oversized-line pass-through is intentional: this optimization does not truncate arbitrary caller text or guarantee that every oversized blocked-skill document is removed. The user guide and owning structure document describe that boundary.
Checklist
Summary by CodeRabbit
Bug Fixes
Documentation
Tests