-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix(claude-desktop): say that first-party exists when gateway is applied #5328
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
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 | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -18,6 +18,7 @@ import { | |||||||||||
| import { inspectDesktop3pConfigLibrary, removeDesktop3pStandardPivot, writeDesktop3pConfig, type Desktop3pConfigMode, parseDesktop3pModeArgs } from "../claude/desktop-3p"; | ||||||||||||
| import { | ||||||||||||
| applyDesktopFirstParty, | ||||||||||||
| isClaudeDesktopMode, | ||||||||||||
| recordClaudeDesktopMode, | ||||||||||||
| removeDesktopFirstParty, | ||||||||||||
| resolveClaudeDesktopApplyMode, | ||||||||||||
|
|
@@ -196,6 +197,44 @@ export function parseDesktopApplyArgs( | |||||||||||
| return { target: { kind, mode: parsedMode.mode } }; | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| /** | ||||||||||||
| * Why a gateway apply happened when the help text calls first-party the default. | ||||||||||||
| * | ||||||||||||
| * `resolveClaudeDesktopMode` keeps an existing install where it is: an explicit | ||||||||||||
| * `claudeCode.desktopMode` wins, and a stored gateway apply marker keeps gateway. Both rules are | ||||||||||||
| * right — a working Desktop install must not flip underneath its user because a default moved. | ||||||||||||
| * Together they mean an existing gateway user never arrives at first-party without discovering | ||||||||||||
| * `--first-party` unaided, while `ocx claude desktop --help` tells them first-party is "(default)". | ||||||||||||
| * | ||||||||||||
| * The fix is not to change the resolution. It is to say, at the moment of the apply, that the | ||||||||||||
| * other mode exists and what selects it. Returns null when the user asked for gateway explicitly, | ||||||||||||
| * because they already know, and when first-party is simply unavailable here — a connected client | ||||||||||||
| * or a disabled intercept cannot run it, so offering it would be advice that fails. | ||||||||||||
| */ | ||||||||||||
| export function gatewayModeExplanation(input: { | ||||||||||||
| requestedExplicitly: boolean; | ||||||||||||
| config: Pick<OcxConfig, "claudeCode" | "runtimeRole">; | ||||||||||||
| connection?: ClientConnectionState; | ||||||||||||
| }): string[] { | ||||||||||||
| if (input.requestedExplicitly) return []; | ||||||||||||
| const connection = input.connection ?? readClientConnectionState(); | ||||||||||||
| if (connection.kind === "connected") return []; | ||||||||||||
|
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. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: sed -n '195,240p' src/cli/claude-desktop.ts
sed -n '1,150p' src/claude/desktop-first-party.ts
rg -n -C 3 'intercept_disabled|intercept\.enabled|first-party|firstParty|is.*First.*Party|desktopMode' src/cli/claude-desktop.ts src/claude srcRepository: lidge-jun/opencodex Length of output: 50376 🏁 Script executed: #!/bin/bash
rg -n -C 8 'function claudeInterceptEnabled|const claudeInterceptEnabled|export .*claudeInterceptEnabled|gatewayModeExplanation\(|loadConfig\(\)' src/claude src/cli/claude-desktop.ts tests -g '*.ts' -g '*.tsx' | head -n 260Repository: lidge-jun/opencodex Length of output: 23154 Suppress the first-party recommendation when interception is disabled. When 🤖 Prompt for AI Agents |
||||||||||||
| // Only a stored preference is worth explaining. Without one, gateway was chosen because | ||||||||||||
| // first-party cannot run here, and naming an unavailable alternative is advice that fails. | ||||||||||||
| const savedMode = input.config.claudeCode?.desktopMode; | ||||||||||||
| const hasSavedGateway = isClaudeDesktopMode(savedMode) && savedMode === "gateway"; | ||||||||||||
| const hasApplyMarker = input.config.claudeCode?.desktopProfile?.appliedFingerprint !== undefined; | ||||||||||||
| if (!hasSavedGateway && !hasApplyMarker) return []; | ||||||||||||
| const reason = hasSavedGateway | ||||||||||||
| ? "this machine has claudeCode.desktopMode saved as gateway" | ||||||||||||
| : "this machine carries a previous gateway apply"; | ||||||||||||
| return [ | ||||||||||||
| `Applied the gateway profile because ${reason}; an existing install is never switched for you.`, | ||||||||||||
| "First-party keeps Desktop on your claude.ai account and routes only the Code tab through the local proxy:", | ||||||||||||
| " ocx claude desktop apply --first-party", | ||||||||||||
| ]; | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| /** | ||||||||||||
| * First-party apply: settings.json env only. The intercept pair the env points at runs inside | ||||||||||||
| * the hub process, so this is a local-hub operation — a connected client machine cannot reach | ||||||||||||
|
|
@@ -361,6 +400,12 @@ export async function handleClaudeDesktopCommand(argv: string[], deps: ApplyProf | |||||||||||
| console.log("Desktop 앱 설정은 그대로이며, Code 탭의 Claude Code만 로컬 프록시를 거칩니다."); | ||||||||||||
| } else { | ||||||||||||
| console.log(`Claude Desktop gateway 설정을 적용했습니다: ${result.path}`); | ||||||||||||
| for (const line of gatewayModeExplanation({ | ||||||||||||
| requestedExplicitly: applyFlags.some(flag => flag !== "--first-party"), | ||||||||||||
| config: loadConfig(), | ||||||||||||
|
Comment on lines
+403
to
+405
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.
When a fresh local installation falls back to gateway because interception is disabled, Useful? React with 👍 / 👎.
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. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: sed -n '180,245p' src/cli/claude-desktop.ts
sed -n '360,425p' src/cli/claude-desktop.ts
rg -n -C 3 'function applyDesktop|applyDesktop\(|appliedFingerprint|desktopMode|parseDesktopApplyArgs' src tests/claude-integrationRepository: lidge-jun/opencodex Length of output: 50375 🏁 Script executed: #!/bin/bash
printf '%s\n' '--- applyDesktop implementation ---'
sed -n '240,365p' src/cli/claude-desktop.ts
printf '%s\n' '--- imports and dependency definitions ---'
sed -n '1,90p' src/cli/claude-desktop.ts
printf '%s\n' '--- relevant tests and test names ---'
rg -n -C 5 'handleClaudeDesktopCommand|apply-flow|fresh|disconnected|gatewayModeExplanation|applyDesktop' tests/claude-integration/claude-desktop-first-party.test.ts tests/claude-integration/claude-desktop-mode-explanation.test.ts
printf '%s\n' '--- changed-file summary ---'
git diff --stat -- src/cli/claude-desktop.ts tests/claude-integration/claude-desktop-first-party.test.ts tests/claude-integration/claude-desktop-mode-explanation.test.tsRepository: lidge-jun/opencodex Length of output: 26944 🏁 Script executed: #!/bin/bash
printf '%s\n' '--- handler and apply ordering with line numbers ---'
sed -n '272,412p' src/cli/claude-desktop.ts | nl -ba -v272
printf '%s\n' '--- bound persistence helpers ---'
rg -n -C 8 'function saveDesktopMode|const saveDesktopMode|function writeDesktop3pConfig|export function writeDesktop3pConfig|appliedFingerprint' src/cli/claude-desktop.ts src/claude/desktop-3p.ts src/server/management/native-integration-routes.ts
printf '%s\n' '--- test invocation coverage ---'
rg -n 'handleClaudeDesktopCommand|gateway fallback|fresh machine|fresh install|fell back to gateway' tests srcRepository: lidge-jun/opencodex Length of output: 38993 🏁 Script executed: sed -n '150,185p' src/cli/claude-desktop.ts | nl -ba -v150
rg -n -C 12 'function saveDesktopMode|const saveDesktopMode|function writeDesktop3pConfig|export function writeDesktop3pConfig' src/cli/claude-desktop.ts src/claude/desktop-3p.ts
rg -n 'handleClaudeDesktopCommand' tests srcRepository: lidge-jun/opencodex Length of output: 11827 🏁 Script executed: sed -n '70,195p' tests/claude-integration/claude-desktop-cli.test.ts | nl -ba -v70
sed -n '490,525p' tests/claude-integration/claude-desktop-cli.test.ts | nl -ba -v490
sed -n '1,70p' tests/claude-integration/claude-desktop-cli.test.ts | nl -ba -v1Repository: lidge-jun/opencodex Length of output: 13688 Reuse the pre-apply configuration for the gateway explanation.
Keep the initial snapshot and pass it to both calls. Add a regression test for this fresh-install path. The existing CLI tests invoke the handler, but the no-argument case uses the normal first-party default, while the explicit gateway case suppresses the explanation.
Suggested change
🤖 Prompt for AI Agents |
||||||||||||
| })) { | ||||||||||||
| console.log(line); | ||||||||||||
| } | ||||||||||||
| } | ||||||||||||
| // The write landed; only the bookkeeping marker did not. Saying nothing | ||||||||||||
| // would leave the saved-vs-applied display wrong with no explanation. | ||||||||||||
|
|
||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| import { describe, expect, test } from "bun:test"; | ||
| import { gatewayModeExplanation } from "../../src/cli/claude-desktop"; | ||
| import type { ClientConnectionState } from "../../src/client/state"; | ||
|
|
||
| const disconnected = { kind: "disconnected" } as ClientConnectionState; | ||
| const connected = { kind: "connected" } as unknown as ClientConnectionState; | ||
|
|
||
| describe("gateway apply explains the first-party alternative", () => { | ||
| test("a stored gateway marker is named as the reason, with the command that switches", () => { | ||
| const lines = gatewayModeExplanation({ | ||
| requestedExplicitly: false, | ||
| config: { claudeCode: { desktopProfile: { appliedFingerprint: "abc123" } } }, | ||
| connection: disconnected, | ||
| }); | ||
|
|
||
| expect(lines.join("\n")).toContain("previous gateway apply"); | ||
| expect(lines.join("\n")).toContain("ocx claude desktop apply --first-party"); | ||
| }); | ||
|
|
||
| test("an explicit saved desktopMode is named as itself, not as a leftover marker", () => { | ||
| const lines = gatewayModeExplanation({ | ||
| requestedExplicitly: false, | ||
| config: { claudeCode: { desktopMode: "gateway" } }, | ||
| connection: disconnected, | ||
| }); | ||
|
|
||
| expect(lines.join("\n")).toContain("desktopMode saved as gateway"); | ||
| }); | ||
|
|
||
| test("asking for gateway explicitly says nothing, because the user already chose", () => { | ||
| expect(gatewayModeExplanation({ | ||
| requestedExplicitly: true, | ||
| config: { claudeCode: { desktopProfile: { appliedFingerprint: "abc123" } } }, | ||
| connection: disconnected, | ||
| })).toEqual([]); | ||
| }); | ||
|
|
||
| test("a connected client says nothing, because first-party cannot run there", () => { | ||
| expect(gatewayModeExplanation({ | ||
| requestedExplicitly: false, | ||
| config: { claudeCode: { desktopProfile: { appliedFingerprint: "abc123" } } }, | ||
| connection: connected, | ||
| })).toEqual([]); | ||
| }); | ||
|
|
||
| test("a fresh machine that fell back to gateway is not told to switch to something unavailable", () => { | ||
| expect(gatewayModeExplanation({ | ||
| requestedExplicitly: false, | ||
| config: {}, | ||
| connection: disconnected, | ||
| })).toEqual([]); | ||
| }); | ||
| }); |
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.
On an existing gateway installation with
claudeCode.intercept.enabled: false, a bare apply passes this connection check and the saved gateway marker causes the helper to recommendocx claude desktop apply --first-party. That command deterministically fails withintercept_disabled, despite the function documentation promising not to offer unavailable first-party mode. CheckclaudeInterceptEnabled(input.config)before returning the explanation; this also covers configurations whose runtime role cannot host the intercept proxy.Useful? React with 👍 / 👎.