test(claude): bind picker proxies without port probes

[Ingwannu]

Summary

• replace the Claude Desktop picker route fixture's probe-and-release consecutive port allocator with a narrow lifecycle CONNECT-factory seam;
• keep production adjacent-port selection unchanged while tests bind the real CONNECT handlers directly on kernel-assigned ports;
• assert both requested production ports are still derived, every real listener is bound to a positive distinct port, and the picker/controller exists before policy assertions run;
• retain the original HTTPS egress and callerAddedTrust 409/trust-cleanup coverage.

Closes #6014.

Why

The old fixture closed both probe sockets before runtime startup. The lifecycle creates its own ephemeral TLS listener before binding the CONNECT pair, so either that listener or another process could claim a just-released port. A picker bind failure correctly degrades to no controller and returns 503, causing the test to miss the 409 policy path it intended to prove.

Keeping reservation sockets open would block the runtime's real binds. The selected seam instead changes only the test allocation mechanism: production still calls the same factory with configured adjacent ports, while the test factory forwards the unchanged handler options to startConnectProxy(0, ...) and keeps those actual sockets owned until normal teardown.

Validation

All local commands ran sequentially in disposable homes under systemd limits (CPUQuota=75%, MemoryMax=1536M, no swap; lighter checks at CPUQuota=50%, MemoryMax=512M):

• bun test tests/claude-integration/claude-desktop-picker-routes.test.ts — 9 pass, 0 fail;
• original callerAddedTrust failure case repeated 10 times — 10/10 pass;
• bun run structure:check — pass;
• bun run privacy:scan — pass;
• bun scripts/file-size-ratchet.ts — pass.

No full suite, full build, or root typecheck was run locally.

Summary by CodeRabbit

• Documentation
  • Clarified that production picker mode uses configured adjacent ports; lifecycle tests use operating-system-assigned ports without changing production port selection.
• Tests
  • Updated picker integration coverage to verify that the runtime and picker proxy use distinct, valid ports and that the Desktop profile uses the picker proxy’s actual port.

[github-actions]

✅ Deterministic PR hygiene checks passed.

[coderabbitai]

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: dbd32f68-0156-4c5c-bf8d-80258cae6ab4

📥 Commits

Reviewing files that changed from the base of the PR and between e69347a and 46535bc.

📒 Files selected for processing (3)

• src/claude/intercept/runtime.ts
• structure/clients/claude-desktop.md
• tests/claude-integration/claude-desktop-picker-routes.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

The intercept runtime now accepts an injectable proxy-start function for both proxy endpoints. The Claude Desktop picker route test uses that seam to bind proxy handlers on OS-assigned ports and verifies that the applied profile uses the picker proxy’s actual port.

Changes

Claude picker proxy startup

Layer / File(s)	Summary
Proxy startup injection src/claude/intercept/runtime.ts:120-193	StartClaudeInterceptOptions adds an optional startProxy function. The runtime uses it for the main intercept proxy and picker proxy, with startConnectProxy as the default.
Picker route fixture and port assertions tests/claude-integration/claude-desktop-picker-routes.test.ts:3-148, structure/clients/claude-desktop.md:155-158	The test replaces port-pair probing with an injected proxy starter that binds on port 0. It checks the requested ports and actual listener ports, then verifies that the profile uses the picker proxy’s actual port. The documentation describes the test setup and production port selection.

Priority: ⬇️ Low

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

Change: Other · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to 46535

No actionable merge-blocking risk is identified; the change is ready for normal merge checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 46535

The test now binds real proxy listeners on operating-system-assigned ports. The normal startup path still uses the existing loopback proxy and authentication controls, and no new exposure was established. Use by callers outside this repository and complete security coverage remain unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The new hook can select the implementation of both local CONNECT listeners, but no repository production caller supplies one. The demonstrated override is confined to the integration fixture; use by external consumers is unknown.

Trust Boundaries and Controls

• observed — The default starter receives the main proxy's token callback and binds locally. The fixture passes those options through rather than removing the control; the separately unauthenticated picker relay is an existing boundary choice.

Resilience and Maintainability Implications

• observed — The fixture now requires the picker runtime and proxy to exist before its route assertions and checks that all three bound listener ports are positive and distinct.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately and concisely describes the main change: updating the Claude picker proxy test to bind proxies without port probing.
Linked Issues check	✅ Passed	Issue #6014 requires a test seam that avoids probe-and-release allocation, preserves production binding, and tests the controller's 409 refusal with trust cleanup. src/claude/intercept/runtime.ts ad…
Out of Scope Changes check	✅ Passed	The reviewed changes stay within Issue #6014. The runtime option provides the required test-only binding seam. The integration fixture changes remove probe-and-release allocation and add assertions fo…

Full details: Docstring Coverage

Explanation

Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[lidge-jun]

리뷰 · 우선순위 36 / 80

이 PR은 Claude Desktop 피커 테스트가 포트를 잠깐 열었다가 닫던 방식을 바꿉니다.

예전 테스트는 빈 포트 두 개를 찾아 연 다음 바로 닫았습니다. 닫힌 포트는 비어 있습니다. 서버는 그 다음에 TLS 리스너를 먼저 만듭니다. 그 리스너나 다른 프로세스가 방금 닫은 포트를 가져가면 피커 프록시가 열리지 않습니다. 프록시가 없으면 컨트롤러도 없습니다. 테스트가 보려던 409 대신 503이 나옵니다. #6006 CI에서 이 실패가 났습니다.

이번 코드는 서버 시작 함수에 startProxy 구멍을 넣습니다. 호출자가 이 구멍을 비워 두면 프로덕션은 계산된 포트로 startConnectProxy를 부릅니다. 테스트는 계산된 번호만 적고, 실제 소켓은 포트 0으로 엽니다. 커널이 번호를 정하고, 테스트가 끝날 때까지 소켓을 들고 있습니다. 계산된 번호가 45600과 45601인지 확인합니다. 실제로 열린 TLS 리스너, 코드용 프록시, 피커 프록시가 서로 다른 양수인지도 확인합니다. 데스크톱 프로필 주소는 피커 프록시가 받은 포트를 씁니다. 409와 인증서 신뢰 정리 확인은 그대로입니다. 베이스는 dev입니다. #6014를 닫는 다른 열린 PR은 없습니다.

라인 - tests/claude-integration/claude-desktop-picker-routes.test.ts 23행 LISTENER_PORT, 84행 boundPorts. 피커의 claude.ai 리스너는 가짜이고 번호가 45679로 고정입니다. 리눅스는 빈 포트를 보통 32768번부터 60999번 사이에서 골라 줍니다. 45679는 그 안에 있습니다. 84행 검사는 진짜로 연 포트 세 개만 서로 다른지 봅니다. 커널이 피커 프록시에 45679를 주면 검사는 통과합니다. 켜진 상태의 selectTunnel도 intercept 포트로 45679를 돌려줍니다. 가짜 리스너와 진짜 프록시가 같은 번호를 씁니다. 이 파일에는 그 번호로 접속하는 검사가 없습니다.

라인 - structure/clients/claude-desktop.md 155행. 여기에는 프로덕션이 항상 설정된 인접 포트를 쓴다고 적혀 있습니다. claudeInterceptProxyPort는 설정 포트가 없으면 공개 포트에 100을 더합니다. 테스트가 CONNECT 팩토리만 넣는다는 말도 넓습니다. 같은 테스트는 피커 리스너, 키체인, 플랫폼도 가짜로 넣습니다. 이 문장은 제품 동작 설명 한가운데 있고, 테스트 주석 66행에 같은 내용이 있습니다.

메인테이너의 판단이 필요한 지점

tests/claude-integration/claude-picker-runtime.test.ts 266행 freePortPair는 아직 포트를 열었다가 닫습니다. 331행은 프록시가 그 번호와 바로 다음 번호에 실제로 붙는지 확인합니다. 이번 PR의 포트 0 구멍을 그 파일에 복사하면 331행이 깨집니다. 그 레이스를 이번 PR에 넣을지, 다음 이슈로 남길지 정해야 합니다.

너의 추천

라우트 테스트의 방향은 맞습니다. 45679를 커널이 골라 주는 구간 밖으로 옮기세요. boundPorts에 그 번호가 있으면 실패하게 하세요. structure/clients/claude-desktop.md 155행부터 158행은 빼세요. claude-picker-runtime.test.ts는 이 PR 밖에 두세요. 그 파일은 실제 인접 포트를 확인해야 합니다. 베이스는 dev로 두세요. 닫을 중복 PR은 없습니다.

이 댓글은 grok-bot이 작성했습니다

[Ingwannu]

Exact head 46535bc496 is fully green: CodeRabbit, all four test shards, gates, desktop shell, structure/storage/API and packaging checks, hygiene, React Doctor and aggregate ci passed. Local capped evidence also includes the full picker-route file 9/9 and the original failing callerAddedTrust case 10/10. @lidge-jun please review/approve; this closes #6014 without changing production port selection.

[lidge-jun]

Landed on dev in #6059 (merge 8923ad9835) as one squashed commit that keeps your authorship. Thank you. Closing because this repository merges into dev, so GitHub does not close carried PRs automatically.