test(claude): cover bounded skill marker paths and document pass-through by luvs01 · Pull Request #5606 · lidge-jun/opencodex

luvs01 · 2026-09-22T20:40:54Z

Summary

The source fix (bounding the bundled-skill marker directory probe to 4,096 UTF-16 code units plus one newline-detection character) already landed on dev via #5619, so this PR now carries only the coverage and documentation portion: exact 4,096/4,097 boundary tests for POSIX and Windows separators, a newline-free oversized carrier case, and the matching docs notes. The default claude-api blocklist, explicit empty-list opt-out, 10,000-character payload threshold and native Anthropic passthrough are unchanged.

This carries only the Claude test/docs 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

Head bfa1b08dab rebased onto current dev; the earlier fix commit was dropped during rebase because its patch contents are already upstream.
bun test tests/claude-integration/claude-inbound.test.ts: 52 pass, 0 fail (202 expect() calls).

Summary by CodeRabbit

Bug Fixes
- Improved handling of bundled skill text when directory markers use different capitalization or path formats.
- Skill markers up to 4,096 characters are now handled consistently; longer lines are preserved unchanged, including lines without a trailing newline.
Documentation
- Added guidance describing the bundled-skill text handling limits and preservation behavior.
Tests
- Added coverage for path-length boundaries, path separators, capitalization, and oversized unterminated lines.

coderabbitai · 2026-09-22T20:41:01Z

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 configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 0d4a3c9d-0fc7-4221-871d-9a8a96acb399

📥 Commits

Reviewing files that changed from the base of the PR and between 305d539 and bfa1b08.

📒 Files selected for processing (3)

docs-site/src/content/docs/guides/claude-code.md
structure/clients/claude-desktop.md
tests/claude-integration/claude-inbound.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

Walkthrough

The changes add integration tests for the 4,096-character skill-directory path boundary and for a long first line without a newline. Documentation describes the bounded probe and the preservation of longer lines.

Changes

Skill-text elision

Layer / File(s)	Summary
Boundary tests and documentation `tests/claude-integration/claude-inbound.test.ts`, `docs-site/src/content/docs/guides/claude-code.md`, `structure/clients/claude-desktop.md`	Tests cover 4,096- and 4,097-character paths with POSIX and Windows separators, plus a long first line without a newline. Documentation describes the 4,096-code-unit probe limit and preservation of longer lines. It also records the unchanged payload threshold and blocked-skills policy.

Priority: ⬇️ Low

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to bfa1b

No actionable merge-blocking issue remains from this review. Normal checks can proceed before the draft is made ready.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (2 skipped: 2 …
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 accurately describes the tests and documentation changes, but it does not state the primary change: bounding Claude skill-marker path inspection.

✨ Finishing Touches

📝 Generate docstrings

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

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 · 2026-09-22T20:45:13Z

✅ Deterministic PR hygiene checks passed.

lidge-jun · 2026-09-22T20:50:38Z

리뷰 · 우선순위 44 / 80

이 PR은 Claude Code가 보낸 “스킬 문서 묶음” 텍스트를 줄일 때, 첫 줄에 있는 경로를 끝없이 읽지 않도록 막아 줍니다. 라우팅된(Anthropic이 아닌) 모델로 갈 때 Base directory for this skill: …로 시작하는 긴 텍스트에서, 경로 줄이 막힌 스킬 이름(기본 claude-api)과 같으면 짧은 안내문으로 바꿉니다. 예전에는 그 줄의 첫 줄바꿈까지 indexOf("\n")로 통째로 찾았습니다. 줄이 아주 길거나 줄바꿈이 없으면 한 번에 너무 많이 훑을 수 있습니다. 지금은 경로 부분만 UTF-16 코드 단위 4,096까지 보고, 줄바꿈을 알기 위해 한 글자를 더 봅니다. 그 안에서 줄이 끝나지 않고 이미 한도를 넘으면 원문을 그대로 둡니다. 경로 끝의 /·\도 벗긴 뒤 마지막 폴더 이름을 비교해서, 테스트에 나온 것처럼 …/claude-api/ 형태도 막힙니다. 문서(가이드·structure)와 4,096/4,097·줄바꿈 없는 큰 경로 테스트가 같이 옵니다. 파일은 4개(+48/−4)이고 base는 dev입니다. #5310의 Claude 경로 조각만 가져 온 것이고, Kiro는 #5553, Fernet은 따로라서 이 PR만으로 #5310 전체를 대체하진 않습니다. Draft이며 작성자 로컬 포커스 테스트 52개는 통과했다고 적혀 있습니다.

라인 - src/claude/inbound.ts maybeElideSkillText — 경로 첫 줄이 4,096을 넘거나, 그 창 안에 줄바꿈이 없으면 검사하지 않고 원문 통과입니다. 막힌 스킬 문서가 일부러/우연히 긴 경로 줄이면 라우팅 모델에 큰 묶음이 그대로 갈 수 있습니다. 작성자도 문서에 “자르거나 전부 제거를 보장하지 않는다”고 적어 두었습니다.

라인 - SKILL_TEXT_PATH_MAX_CHARS 이름 — 상수 이름은 CHARS인데 가이드·structure는 “UTF-16 code units”라고 합니다. JS length/slice와는 맞지만, 나중에 읽는 사람이 “글자 수”로 오해할 수 있습니다.

라인 - 끝 슬래시 제거 후 lastIndexOf("/") — 예전 split+pop과 흔한 POSIX/Windows/UNC는 같고, 끝 구분자 있는 경로를 더 잘 처리합니다. 동작 변경 폭은 작아 보입니다.

라인 - #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이 작성했습니다

chatgpt-codex-connector · 2026-09-23T00:04:54Z

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.

…riptions, launchd levers, passthrough tool ids (#5678) * test(claude): cover bounded skill marker paths and document pass-through Carries #5606. Adds the exact 4,096/4,097 UTF-16 code unit boundary cases for POSIX and Windows skill directory markers and a long newline-free carrier, and documents that a longer directory line is sent unchanged. The seven translated Claude Code guides now state the same pass-through rule as the English guide. Supersedes #5606. Co-authored-by: Epinephrine <27862058+luvs01@users.noreply.github.com> * fix(system-env): refresh and drop the launchd levers opencodex owns Carries #5622. A lever opencodex already tracks as injected is refreshed instead of skipped, a tracked lever the current config no longer produces is unset, and PUT /api/claude-code reconciles on every model slot and lever field, not only systemEnv and authMode. A launchd value the user set before injection is never touched. The management API structure note records the PUT reconciliation. Supersedes #5622. Co-authored-by: terin <100397903+sh940701@users.noreply.github.com> * feat(claude): describe routed models instead of "From gateway" Carries #5621. Readable Claude Code /v1/models rows carry a description naming the native model or provider/model they route to, which Claude Code 2.1.257 and later shows in the picker; the 1M copy keeps it and a Fast sibling appends " · Fast". The gateway-model cache preserves string descriptions. The Claude Desktop structure note records the picker description contract. Supersedes #5621. Co-authored-by: terin <100397903+sh940701@users.noreply.github.com> * fix(claude): keep routed model windows without disabling compact Carries #5665. Claude Code aliases are minted as ocx-claude-/ocx-claude2- so the picker keeps them while Claude Code applies CLAUDE_CODE_MAX_CONTEXT_TOKENS without DISABLE_COMPACT; saved claude-ocx-/claude-ocx2- ids keep decoding. maxContextTokens now injects only the window. The gateway cache mirrors the picker's contains-claude rule and keeps #5621's descriptions. Folded review fixes: native fallback recognizes the current prefixes; the context-window map keeps registering the legacy spellings at the same window; no compact lever is ever derived from maxContextTokens (pinned for values outside the 100k-1M compact range); the tracked DISABLE_COMPACT from older releases is removed by the system-env produced-key sweep instead of a one-off cleanup; the Claude Desktop structure note records the alias and env contract. Supersedes #5665. Co-authored-by: terin <100397903+sh940701@users.noreply.github.com> * fix(claude): sanitize tool_use ids on native Anthropic passthrough Carries #5628. The native passthrough bypasses the Anthropic adapter, so tool call ids minted by routed models (Devin's Bash:0#<hex>) reached api.anthropic.com verbatim and 400ed. The request-scoped allocator now rewrites non-conforming and overlength ids, keeps call/result pairing, and leaves conforming ids byte-identical. Folded review fixes: an empty id fails locally with a 400 before the upstream fetch instead of being forwarded; regressions cover the empty id, an overlength id and a collision with an existing valid id; the Claude Desktop structure note and the Claude Code guide in all eight languages describe the id rewrite. Supersedes #5628. Co-authored-by: wuwei <27188611+Haven2026@users.noreply.github.com> * fix(claude): keep legacy slot selectors and hand-edited DISABLE_COMPACT safe on upgrade Folds the bundle's adversarial review into the #5665 and #5622 carries. A legacy claude-ocx-/claude-ocx2- selector configured in an OpenCodex model slot is emitted in its current ocx-claude spelling on every env path. The route is identical, and Claude Code then applies the configured window instead of falling back to 200k accounting now that DISABLE_COMPACT is no longer paired with maxContextTokens. A selection saved by Claude Code's own picker stays a documented re-pick. The system-env sweep removes a tracked DISABLE_COMPACT only while it still holds the 1 older releases injected; a value the user changed by hand is released from tracking without being deleted. Co-authored-by: terin <100397903+sh940701@users.noreply.github.com> * fix(claude): keep legacy picker selectors on connected clients and Fable passthrough Folds the Codex and CodeRabbit review of the bundle. A legacy claude-ocx-native--claude-fable-* picker value compared only against the new ocx-claude spelling and fell off the native Anthropic passthrough; both spellings are accepted again, with the legacy value back in the endpoint test. The connected-client context-window map registers the legacy route and native spellings like the local map does, so a saved legacy selector keeps its [1m] subagent marker on a connected hub. Co-authored-by: terin <100397903+sh940701@users.noreply.github.com> --------- Co-authored-by: Epinephrine <27862058+luvs01@users.noreply.github.com> Co-authored-by: terin <100397903+sh940701@users.noreply.github.com> Co-authored-by: wuwei <27188611+Haven2026@users.noreply.github.com>

lidge-jun · 2026-09-23T11:50:21Z

Carried onto dev in bundle PR #5678 (squash-merged as 9d1fa87), rebuilt on current dev as commit f50f33d on the lane branch with a Co-authored-by trailer for you, so the credit stays on the merged commit. Closing this one as superseded. Thank you for the fix.

github-actions Bot added the bug Something isn't working label Sep 22, 2026

luvs01 marked this pull request as ready for review September 23, 2026 00:04

luvs01 requested review from Ingwannu and lidge-jun as code owners September 23, 2026 00:04

test(claude): cover bounded skill marker paths and document pass-through

bfa1b08

luvs01 force-pushed the stack/claude-skill-marker-bounds branch from 305d539 to bfa1b08 Compare September 23, 2026 09:23

luvs01 changed the title ~~fix(claude): bound bundled-skill marker path inspection~~ test(claude): cover bounded skill marker paths and document pass-through Sep 23, 2026

github-actions Bot added chore Maintenance, CI, tests, refactors, or build changes (not a user-facing bug or feature). and removed bug Something isn't working labels Sep 23, 2026

lidge-jun mentioned this pull request Sep 23, 2026

fix(claude): bundle lane C — routed windows with compact, picker descriptions, launchd levers, passthrough tool ids #5678

Merged

3 tasks

lidge-jun closed this Sep 23, 2026

Navigation Menu

test(claude): cover bounded skill marker paths and document pass-through - #5606

test(claude): cover bounded skill marker paths and document pass-through#5606
luvs01 wants to merge 1 commit into
devfrom
stack/claude-skill-marker-bounds

luvs01 commented Sep 22, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Sep 22, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

github-actions Bot commented Sep 22, 2026

Uh oh!

lidge-jun commented Sep 22, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 23, 2026

Uh oh!

lidge-jun commented Sep 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

luvs01 commented Sep 22, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Verification

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

github-actions Bot commented Sep 22, 2026

Uh oh!

lidge-jun commented Sep 22, 2026

리뷰 · 우선순위 44 / 80

Uh oh!

chatgpt-codex-connector Bot commented Sep 23, 2026

Uh oh!

lidge-jun commented Sep 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

luvs01 commented Sep 22, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Sep 22, 2026 •

edited

Loading