-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix(claude): pin Desktop mode when disabling CLI first-party routing #6046
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
1a871c4
efebe4d
1788ae4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -200,7 +200,7 @@ per-request first-party callback reads that live object; a failed write leaves i | |
| | Effort and fallback | `src/server/management/agent-settings-routes.ts` — `GET/PUT /api/effort-caps`, `/api/subagent-models`, `/api/subagent-model-fallback`. Caps clamp; they do not reject. | | ||
| | Grok and Claude integrations | `src/server/management/agent-settings-routes.ts` — `GET /api/grok`, `PUT /api/grok/selection`, `POST /api/grok/apply`, `GET/PUT /api/claude-desktop`, `POST /api/claude-desktop/apply` (`mode`: `gateway` default for new installs, `first-party` opt-in, or legacy shapes), `GET /api/claude-desktop/status` (`mode`, `riskWarning`, `firstParty`), `GET/PUT /api/claude-code`. Gateway apply writes an external app's profile, so its status probe must read the same resolved path it writes (see [`responses.md`](transports/responses.md)); first-party apply writes only the Claude Code proxy env, see [`clients/claude-desktop.md`](clients/claude-desktop.md#desktop-modes-gateway-and-first-party). `gui/src/pages/ClaudeDesktop.tsx` renders the mode selector and sends the chosen `mode` with apply. `PUT /api/claude-code` re-runs macOS system-env reconciliation (`src/server/system-env.ts`) whenever the body carries `systemEnv`, `authMode`, a model slot or a lever field: keys opencodex tracks as injected are refreshed or unset once the config stops producing them, and a launchd value the user set before injection is never touched. | | ||
|
|
||
| `GET /api/claude-code` reports `cliFirstParty`, `desktopFirstParty`, `cliFirstPartyApplied`, `interceptEligible`, `interceptRunning`, and the eight-value `sharedProxy: FirstPartyProxyStatus` from observed settings and the bound listener. `interceptEligible = claudeInterceptEnabled(config)` uses the same GET snapshot as `sharedProxy` and `interceptRunning`; the latter remains bound listener present AND eligible. The ordered classifier gives unreadable → `unknown`, absent or non-loopback URL → `none`, foreign CA with an opencodex token → `foreign`, foreign CA with a tokenless loopback URL → `local`, no bound listener → `stopped`, ineligible applied settings at the bound port → `disabled`, other ineligible or stale/mismatched settings → `broken`, and eligible applied settings at the bound port → `live`. `cliFirstPartyApplied` requires CLI intent and `live`. `PUT /api/claude-code` accepts standalone `cliFirstParty`; CLI-on repeats eligibility and port checks inside the locked persisted mutation, reconciles, and conditionally rolls back its own fields on failure. CLI-off deletes intent but retains an env Desktop still desires. A successful nothing-desired reconcile returns `settings_residual` for every status except `none`, including `local`; unreadable cleanup returns the coded 500. `enabled:false` alone leaves the env untouched, and a mixed body returns 400 before save. GUI normalization maps only missing `sharedProxy:undefined` to `none`, invalid statuses including `null` to `unknown`; `interceptEligible:undefined` from an older cache maps to `true`, while present values use `=== true`. The source coverage map checks all eight statuses. Notice order is unknown, foreign, local, residual when undesired, disabled, routingOff for stopped/broken with ineligible routing, stopped, broken, notApplied, shared, null. Unknown copy states uncertainty, local copy names the unconfirmed 127.0.0.1 proxy and manual HTTPS_PROXY removal, disabled copy retains the first-party-off remedy, foreign copy directs manual CA/proxy repair, routingOff says to restore Claude routing or turn first-party off, stopped says to start opencodex, and eligible broken advises `ocx ensure` or restart. | ||
| `GET /api/claude-code` reports `cliFirstParty`, `desktopFirstParty`, `cliFirstPartyApplied`, `interceptEligible`, `interceptRunning`, and the eight-value `sharedProxy: FirstPartyProxyStatus` from observed settings and the bound listener. `interceptEligible = claudeInterceptEnabled(config)` uses the same GET snapshot as `sharedProxy` and `interceptRunning`; the latter remains bound listener present AND eligible. The ordered classifier gives unreadable → `unknown`, absent or non-loopback URL → `none`, foreign CA with an opencodex token → `foreign`, foreign CA with a tokenless loopback URL → `local`, no bound listener → `stopped`, ineligible applied settings at the bound port → `disabled`, other ineligible or stale/mismatched settings → `broken`, and eligible applied settings at the bound port → `live`. `cliFirstPartyApplied` requires CLI intent and `live`. `PUT /api/claude-code` accepts standalone `cliFirstParty`; CLI-on repeats eligibility and port checks inside the locked persisted mutation, reconciles, and conditionally rolls back its own fields on failure. CLI-off deletes intent, pins an absent `desktopMode` from the post-clear observation, and retains an env Desktop still desires; when that retained env's ownership was ambiguous (an owned shared proxy suppressed by the flag), the success response warns `shared_proxy_retained` so the operator can pin `gateway` explicitly to release it. A successful nothing-desired reconcile returns `settings_residual` for every status except `none`, including `local`; unreadable cleanup returns the coded 500. `enabled:false` alone leaves the env untouched, and a mixed body returns 400 before save. GUI normalization maps only missing `sharedProxy:undefined` to `none`, invalid statuses including `null` to `unknown`; `interceptEligible:undefined` from an older cache maps to `true`, while present values use `=== true`. The source coverage map checks all eight statuses. Notice order is unknown, foreign, local, residual when undesired, disabled, routingOff for stopped/broken with ineligible routing, stopped, broken, notApplied, shared, null. Unknown copy states uncertainty, local copy names the unconfirmed 127.0.0.1 proxy and manual HTTPS_PROXY removal, disabled copy retains the first-party-off remedy, foreign copy directs manual CA/proxy repair, routingOff says to restore Claude routing or turn first-party off, stopped says to start opencodex, and eligible broken advises `ocx ensure` or restart. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: git diff --name-only e807e1e27be7dc2a3933739f1644cd3b22dff748 1788ae43208b2ed81e3ea7ad5779e9ef1d66a04c
rg -n 'docs-site|user-visible behavior|configuration' AGENTS.md structure/AGENTS.md docs-site/AGENTS.md
rg -n 'shared_proxy_retained|cliFirstParty|desktopMode' docs-site | head -80Repository: lidge-jun/opencodex Length of output: 3106 Document the CLI opt-out change in
🤖 Prompt for AI Agents |
||
|
|
||
| | File-integration plans | `src/server/management/integration-routes.ts` and `aside-profile-routes.ts` — `POST /api/client-integrations/preview`, `POST /api/client-integrations/restore/preview`, and `POST /api/client-integrations/aside/profiles/{profileId}/preview`. Management-authenticated, declared non-mutating, and they write nothing: no snapshot, no lock, no maintenance, no recovery. They answer `409 integration_preview_unavailable` rather than gathering a model roster, because discovery refreshes credentials and writes the provider cache. Responses carry only declared managed schema paths, closed change kinds and an opaque fingerprint; no value, filesystem location or selected member identity appears. Mutation routes accept `operation` and `planFingerprint` together or not at all, reject a half-bound request and an operation that disagrees with the change, and answer `409 integration_preview_stale` with a freshly computed plan. Binding is an optimistic token, never authorization. [The integration contract](clients/integrations.md) owns the ordering. | | ||
| | Grok reset coupons | `src/server/management/grok-coupon-routes.ts` — `GET /api/grok/reset-coupons`, `POST /api/grok/reset-coupons/consume`. The dashboard owner is `gui/src/hooks/useGrokResetCoupons.ts` with `gui/src/components/provider-workspace/GrokResetCoupons.tsx`, wired into the xAI OAuth rows of `ProviderAuthPanel`. Redemption truth is the settled ledger `code`, not the HTTP status: a replayed failure returns 200 with `replayed: true`. See [`providers/xai-grok.md`](providers/xai-grok.md). | | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: lidge-jun/opencodex
Length of output: 2426
🏁 Script executed:
Repository: lidge-jun/opencodex
Length of output: 42541
🏁 Script executed:
Repository: lidge-jun/opencodex
Length of output: 42469
Base
shared_proxy_retainedon the reconciled state.When Desktop integration is disabled,
resolveClaudeDesktopModecan still return"first-party"from owned settings. The route then setsretainedAmbiguousbefore reconciliation. After it removescliFirstParty,firstPartyDesiredrequests neither Desktop nor CLI first-party settings, so reconciliation removes the proxy. The response still emitsshared_proxy_retained.Use the reconciliation result before adding the warning.
Suggested fix
🤖 Prompt for AI Agents