Repository navigation
feat(claude-desktop): first-party mode as default, gateway profile as explicit opt-in - #5302
Conversation
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Repository: lidge-jun/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. |
|
E2E-tested the mode switch against a real local backend + dashboard (isolated OpenCodex / Claude settings / Desktop library / Codex dirs, on-disk verification). Found & fixed in Retest (PR3 GUI + this fix): First-party → Gateway → First-party round trip works directly in the GUI and via
Also verified (first run)CONNECT proxy on 10200 tunnels Not tested: real Claude Desktop Code-tab/subagent inference (no Desktop installed), forced gateway-cleanup failure branch, disabled/stopped-proxy UI variants. |
a59cecc to
d354361
Compare
리뷰 · 우선순위 56 / 80이 PR은 Claude Desktop을 opencodex에 붙이는 방식을 둘로 나눕니다. 기본은 first-party입니다. Desktop 앱 자체(로그인, Chat, connectors, Remote Control)는 claude.ai에 그대로 두고, Code 탭이 띄우는 Claude Code(그리고 서브에이전트·단독 라인 - 메인테이너의 판단이 필요한 지점 네이티브 토글을 “ON/OFF만”으로 둘지, apply와 같이 gateway→first-party 교체까지 맡길지. 토글이 모드 전환 UI가 아니라면 문서/가드만으로도 충분할 수 있고, GUI(#5303)가 모드를 바꾸면 토글 패리티 구멍이 실제 사고로 이어질 수 있습니다. 또한 #5301 없이 이 브랜치만 너의 추천 #5301이 먼저 안정된 뒤 이 PR을 머지하는 스택 순서를 유지하세요. 머지 전에 네이티브 토글 first-party enable에 gateway pivot(+ 가능하면 mode 마커 저장)을 apply와 맞추거나, 토글은 모드 전환을 하지 않는다고 명시하고 잔여 gateway가 있으면 거부하는 쪽 중 하나를 고르세요. gateway apply의 mode 마커 실패도 first-party처럼 warning으로 노출하는 편이 낫습니다. #5303과 함께 E2E로 모드 왕복(gateway↔first-party)과 토글 OFF 잔여 정리를 한 번 더 보면 좋습니다. 이 댓글은 grok-bot이 작성했습니다 |
⏳ DRAFT
What to do
Review readiness checklist
0/4 boxes ticked. This PR stays in draft until every box above is ticked. |
|
Addressed in the latest commit on this branch:
Stack order unchanged: #5301 must land first. |
d354361 to
f89ce7e
Compare
… explicit opt-in Add claudeCode.desktopMode (first-party | gateway). First-party keeps Claude Desktop on claude.ai and writes only HTTPS_PROXY/NODE_EXTRA_CA_CERTS into Claude Code's settings.json so the Code tab, subagents and the claude CLI go through the local intercept pair. Gateway keeps the existing 3P profile writer. - resolveClaudeDesktopMode: explicit > applied gateway fingerprint > first-party, so existing gateway installs do not flip on update while new installs get 1P - resolveClaudeDesktopApplyMode: implied 1P falls back to gateway where the intercept pair cannot run (client role / intercept disabled) - CLI: ocx claude desktop apply [--first-party|--gateway]; legacy shape flags imply --gateway; connected clients default to gateway - API: /api/claude-desktop/apply accepts first-party|gateway (+ legacy shapes), status reports mode + firstParty block; native toggle applies resolved mode and disable removes both gateway profile and 1P env - ocx ensure: refresh stale 1P env when ON, remove it when OFF - modes are mutually exclusive; foreign proxy/CA env is never overwritten Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…file Switching gateway -> first-party from the GUI/CLI/API previously refused with gateway_profile_active and required turning the integration off first, while the docs and the GUI switch note promise a direct replacement in both directions. The first-party branch now pivots the owned gateway profile back to standard (removeDesktop3pStandardPivot with replaceWhileEnabled, since the durable switch stays ON) before writing the env, and fails without writing when the pivot cannot complete. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…for first-party - The native ON toggle pivots an owned gateway profile to standard (replaceWhileEnabled) before writing the first-party env, and persists the desktopMode marker, matching POST /api/claude-desktop/apply. - Gateway apply now reports a failed mode-marker save as saved:false plus a warning instead of dropping the result. - ensure/update warns when an explicit first-party marker contradicts a gateway profile still on disk rather than returning silently. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
f89ce7e to
c9ed363
Compare
추가 리뷰 · 우선순위 42 / 80이 추가 리뷰는 직전 리뷰 이후 tip 라인 - gateway → first-party 전환 후에도 메인테이너의 판단이 필요한 지점 first-party로 바꾼 뒤 gateway 너의 추천 직전 블로커는 해소된 편이라, 머지 전 남은 건 fingerprint 정리(또는 마커 실패 시 롤백/거부)와 토글 gateway 경로의 모드 마커 패리티 정도입니다. #5301 머지·CI 그린·체크리스트 기입을 기다린 뒤 이 PR을 올리고, #5303에서 GUI 모드 왕복 E2E로 한 번 더 확인하는 흐름이 안전합니다. 이 댓글은 grok-bot이 작성했습니다 |
… record gateway mode from the native toggle All three mode-marker writers (CLI apply, /api/claude-desktop/apply, native toggle) now share recordClaudeDesktopMode. Switching to first-party clears desktopProfile.appliedFingerprint/appliedAt so a lost explicit marker can no longer resolve back to gateway while first-party env is on disk; the profile assignments stay for a later gateway apply. The native toggle's gateway enable branch saves desktopMode="gateway" like the apply route does. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
f777c7f addresses the two code points:
Not changed: |
추가 리뷰 · 우선순위 30 / 80이 추가 리뷰는 직전 tip 라인 - 메인테이너의 판단이 필요한 지점
너의 추천 직전 코드 블로커는 해소된 상태입니다. 머지 전에 gateway 토글 마커 테스트를 시작 상태를 비운 채로 고치고, #5301 머지·CI 전부 그린·체크리스트 기입 후 이 PR을 올리는 흐름이면 됩니다. 제품 동작은 이 tip 기준으로 first-party 기본 + gateway 교체 계약이 문서·코드·테스트(약한 gateway 토글 테스트 제외)와 맞춰졌습니다. 이 댓글은 grok-bot이 작성했습니다 |
…er from an unmarked config Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
Good catch — c21865c fixes the weak test. The toggle test now starts from a config with no |
추가 리뷰 · 우선순위 18 / 80이 추가 리뷰는 직전 tip 라인 - tip CI는 리눅스·keyring·gates 등은 통과했지만 macos 1/2·2/2가 아직 pending입니다. 머지 전 전부 그린인지 한 번 더 보면 됩니다. 메인테이너의 판단이 필요한 지점 코드 쪽 리뷰 지적은 이번 tip에서 닫힌 상태입니다. 남은 결정은 스택 순서(#5301 → 이 PR → #5303)와 tip을 너의 추천 제품 동작·계약·테스트(게이트웨이 토글 마커 포함)는 이 tip 기준으로 맞춰졌습니다. #5301 머지 후 base를 이 댓글은 grok-bot이 작성했습니다 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c21865ced0
ℹ️ 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".
| config: Pick<OcxConfig, "claudeCode" | "port">, | ||
| opencodexConfigDir = getConfigDir(), | ||
| ): DesktopFirstPartyTarget { | ||
| const proxyPort = claudeInterceptProxyPort(config, config.port ?? 10100); |
There was a problem hiding this comment.
When the server binds a different port from the persisted config.port—notably an ephemeral port: 0 or an automatic fallback because the configured port is occupied—the intercept runtime derives its listener from the actual bound public port, while this target writes HTTPS_PROXY from the stale persisted value (100 for port: 0). Apply and subsequent ensure/status inspection can therefore report first-party mode as current while every Claude Code request is sent to a dead port. Pass the live/bound proxy port into the first-party apply and inspection paths rather than deriving it solely from persisted configuration.
Useful? React with 👍 / 👎.
| const appliedFingerprint = config.claudeCode?.desktopProfile?.appliedFingerprint ?? null; | ||
| const library = inspectDesktop3pConfigLibrary({ appliedFingerprint }); | ||
| if (library.kind === "gateway_ours" || library.kind === "gateway_drifted") { | ||
| const removed = removeDesktop3pStandardPivot({ appliedFingerprint, replaceWhileEnabled: true }); |
There was a problem hiding this comment.
When switching an existing gateway install to first-party mode, this removes the working gateway profile before applyDesktopFirstParty can reject a foreign/unreadable settings file or fail to write it, leaving the user with neither mode despite the command returning failure. The inverse path at applyDesktop similarly removes the first-party env before catalog discovery and gateway writing can fail, and the management routes repeat both orderings. Preflight the destination and retain or roll back the old mode until the replacement has committed.
AGENTS.md reference: src/AGENTS.md:L15-L17
Useful? React with 👍 / 👎.
| } catch { | ||
| return { ok: false, reason: "ca_unavailable", path: target.caCertPath }; | ||
| } | ||
| const written = applyClaudeInterceptSettings(target.env, options.claudeConfigDir); |
There was a problem hiding this comment.
If the settings directory becomes unwritable, the disk is full, or the atomic rename fails, applyClaudeInterceptSettings throws from its mkdir/write/rename operations rather than returning its declared failure union. This wrapper catches only CA creation, so native-toggle and ensure callers can reject the request/startup reconciliation instead of returning the established refusal result; removeDesktopFirstParty has the same problem. Catch filesystem exceptions around both settings mutations and map them to a bounded failure reason such as write_failed.
AGENTS.md reference: src/AGENTS.md:L15-L17
Useful? React with 👍 / 👎.
|
Superseded by #5319, merged to The chain was carried as a GitHub native stack, which stops accepting ordinary merges once its bottom lands by squash. Its tip already contained both ancestors and branched cleanly from Closing as superseded rather than stale. |
1 participant
Summary
Stack 2/3.
ocx claude desktop apply, the native Claude Desktop toggle, and/api/claude-desktop/applynow default to first-party mode: Claude Desktop itself stays on claude.ai (Chat tab, connectors, cloud sessions, Remote Control untouched) and only the Code tab's Claude Code processes — plus subagents and the standalone CLI — are routed through the intercept proxy from PR 1. The existing whole-app gateway profile is kept as an explicit opt-in (--gateway,mode: "gateway").claudeCode.desktopMode: "first-party" | "gateway". Resolution insrc/claude/desktop-first-party.ts:--first-partyfails with a reason instead.HTTPS_PROXY=http://127.0.0.1:<port+100>andNODE_EXTRA_CA_CERTS=<configDir>/claude-intercept/ca.peminto~/.claude/settings.json.envvia the ownership-aware writer; disable / switching to gateway removes only OpenCodex-owned values. The two modes are mutually exclusive: gateway apply removes first-party env first, first-party apply refuses while a managed gateway profile is active.--first-party | --gateway [--static|--hybrid|--discovery-only]; legacy shape flags imply gateway; conflicting flags rejected. Mode is persisted with a narrowmutatePersistedConfig(no stale whole-config snapshot).applyacceptsmode;statusaddsmodeand afirstPartyblock (applied/stale, intercept enabled/running, proxyPort, caCertPath). Gateway policy-conflict reporting is suppressed in first-party mode since Desktop's own config is not touched.ocx ensurereconciles first-party env (refresh stale, remove when disabled) alongside existing gateway cleanup.gatewayexplicitly where they previously relied on it being the default.Verification
bun run typecheckbun test tests/claude-integration/claude-desktop-first-party.test.ts tests/claude-integration/claude-management-api.test.ts tests/codex-integration/native-claude-desktop-toggle.test.ts tests/clibun run privacy:scan,bun run structure:checkbun run testrunning on the stack tip; will follow up if anything fails.Checklist
Link to Devin session: https://app.devin.ai/sessions/4ec9c3528d424ced9e4a4da906168ad1
Open in Devin Desktop: https://app.devin.ai/desktop/session/4ec9c3528d424ced9e4a4da906168ad1?variant=devin
Requested by: @lidge-jun
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
All CI tests are green on my local testing.
I pushed my PR to the latest dev commit.
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.