Skip to content

Commit 75f1520

Browse files
ChrisPeiclaude
andauthored
feat: add platform provider abstraction for cross-platform support (#80)
* feat: add platform provider abstraction for cross-platform support Introduce a platform provider pattern to centralize all platform-specific logic (Windows, macOS, Linux) in one place. This fixes Windows shell command execution issues and improves maintainability. Changes: - Add platform provider interface and implementations for Windows, macOS, Linux - Refactor cli.ts to use platform provider for CLI installation - Refactor terminal/env.ts to use platform provider for shell detection - Refactor claude/env.ts to use platform provider for environment building - Refactor claude-token.ts to use platform provider for PATH extension - Fix path separator issues (use path.basename instead of split('/')) The platform provider handles: - Shell configuration and detection - PATH building with common tool locations - Environment variable setup - CLI installation/uninstallation - Locale detection Co-Authored-By: Claude <noreply@anthropic.com> * fix: address PR review feedback Based on PR #80 review by @serafimcloud: 1. Windows PATH limitation (Critical): - Remove `setx PATH` which has 1024 char limit and can truncate PATH - CLI install dir is already in buildExtendedPath() for app usage - Return pathHint for users who want terminal access 2. Variable shadowing (Critical): - Rename `path` to `filePath` in chats.ts to avoid shadowing the imported `path` module 3. Electron coupling (Medium): - Remove unused `app` import from windows.ts - Remove unused `exec`, `promisify`, `lstatSync` imports 4. execCommand consistency (Low): - Add documentation explaining execCommand is for simple commands - Complex shell commands (osascript, pipes) may use exec directly Co-Authored-By: Claude <noreply@anthropic.com> * fix(windows): install CLI to ~/.local/bin instead of %LOCALAPPDATA% - Install to ~/.local/bin which is already in buildExtendedPath() - Avoids needing to modify system PATH or prompt user - App can find CLI automatically via extended PATH - Users with ~/.local/bin in their PATH can use it from terminal too Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent e393091 commit 75f1520

12 files changed

Lines changed: 1218 additions & 187 deletions

File tree

src/main/lib/claude-token.ts

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { execSync, spawn } from "node:child_process";
22
import { existsSync, readFileSync } from "node:fs";
33
import { homedir } from "node:os";
44
import { join } from "node:path";
5+
import { buildExtendedPath, isWindows } from "./platform";
56

67
interface ClaudeCredentials {
78
claudeAiOauth?: {
@@ -242,23 +243,14 @@ export function isTokenExpired(expiresAt?: number): boolean {
242243

243244
/**
244245
* Build extended PATH with common installation locations
245-
* This is necessary because when running from Finder/Dock, the PATH
246-
* may not include directories where claude CLI is installed
246+
* This is necessary because when running from Finder/Dock (macOS) or
247+
* Start Menu (Windows), the PATH may not include directories where
248+
* claude CLI is installed
249+
*
250+
* Delegates to platform provider for cross-platform support.
247251
*/
248252
function getExtendedPath(): string {
249-
const home = homedir();
250-
const extendedPaths = [
251-
'/opt/homebrew/bin',
252-
'/usr/local/bin',
253-
`${home}/.local/bin`,
254-
`${home}/.bun/bin`,
255-
`${home}/.cargo/bin`,
256-
'/opt/local/bin',
257-
`${home}/.nvm/versions/node/*/bin`, // Common Node.js installations
258-
].filter(Boolean);
259-
260-
const currentPath = process.env.PATH || '';
261-
return [...extendedPaths, ...currentPath.split(':')].join(':');
253+
return buildExtendedPath(process.env.PATH);
262254
}
263255

264256
/**
@@ -268,7 +260,7 @@ function getExtendedPath(): string {
268260
export function isClaudeCliInstalled(): boolean {
269261
try {
270262
// Use 'where' on Windows, 'which' on Unix-like systems
271-
const command = process.platform === 'win32' ? 'where claude' : 'which claude';
263+
const command = isWindows() ? 'where claude' : 'which claude';
272264
const fullPath = getExtendedPath();
273265

274266
execSync(command, {

src/main/lib/claude/env.ts

Lines changed: 80 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,13 @@ import path from "node:path"
44
import os from "node:os"
55
import { app } from "electron"
66
import { stripVTControlCharacters } from "node:util"
7+
import {
8+
platform,
9+
buildExtendedPath,
10+
getDefaultShell,
11+
isWindows,
12+
isMacOS,
13+
} from "../platform"
714

815
// Cache the shell environment
916
let cachedShellEnv: Record<string, string> | null = null
@@ -37,29 +44,33 @@ export function getBundledClaudeBinaryPath(): string {
3744
}
3845

3946
const isDev = !app.isPackaged
40-
const platform = process.platform
47+
const currentPlatform = process.platform
4148
const arch = process.arch
4249

4350
// Only log verbose info on first call
4451
if (process.env.DEBUG_CLAUDE_BINARY) {
4552
console.log("[claude-binary] ========== BUNDLED BINARY PATH ==========")
4653
console.log("[claude-binary] isDev:", isDev)
47-
console.log("[claude-binary] platform:", platform)
54+
console.log("[claude-binary] platform:", currentPlatform)
4855
console.log("[claude-binary] arch:", arch)
4956
console.log("[claude-binary] appPath:", app.getAppPath())
5057
}
5158

5259
// In dev: apps/desktop/resources/bin/{platform}-{arch}/claude
5360
// In production: {resourcesPath}/bin/claude
5461
const resourcesPath = isDev
55-
? path.join(app.getAppPath(), "resources/bin", `${platform}-${arch}`)
62+
? path.join(
63+
app.getAppPath(),
64+
"resources/bin",
65+
`${currentPlatform}-${arch}`
66+
)
5667
: path.join(process.resourcesPath, "bin")
5768

5869
if (process.env.DEBUG_CLAUDE_BINARY) {
5970
console.log("[claude-binary] resourcesPath:", resourcesPath)
6071
}
6172

62-
const binaryName = platform === "win32" ? "claude.exe" : "claude"
73+
const binaryName = currentPlatform === "win32" ? "claude.exe" : "claude"
6374
const binaryPath = path.join(resourcesPath, binaryName)
6475

6576
if (process.env.DEBUG_CLAUDE_BINARY) {
@@ -71,8 +82,13 @@ export function getBundledClaudeBinaryPath(): string {
7182

7283
// Always log if binary doesn't exist (critical error)
7384
if (!exists) {
74-
console.error("[claude-binary] WARNING: Binary not found at path:", binaryPath)
75-
console.error("[claude-binary] Run 'bun run claude:download' to download it")
85+
console.error(
86+
"[claude-binary] WARNING: Binary not found at path:",
87+
binaryPath
88+
)
89+
console.error(
90+
"[claude-binary] Run 'bun run claude:download' to download it"
91+
)
7692
} else if (process.env.DEBUG_CLAUDE_BINARY) {
7793
const stats = fs.statSync(binaryPath)
7894
const sizeMB = (stats.size / 1024 / 1024).toFixed(1)
@@ -112,16 +128,49 @@ function parseEnvOutput(output: string): Record<string, string> {
112128
}
113129

114130
/**
115-
* Load full shell environment using interactive login shell.
116-
* This captures PATH, HOME, and all shell profile configurations.
131+
* Strip sensitive keys from environment
132+
*/
133+
function stripSensitiveKeys(env: Record<string, string>): void {
134+
for (const key of STRIPPED_ENV_KEYS) {
135+
if (key in env) {
136+
console.log(`[claude-env] Stripped ${key} from shell environment`)
137+
delete env[key]
138+
}
139+
}
140+
}
141+
142+
/**
143+
* Load full shell environment.
144+
* - Windows: Derives PATH from process.env + common install locations (no shell spawn)
145+
* - macOS/Linux: Spawns interactive login shell to capture PATH from shell profiles
117146
* Results are cached for the lifetime of the process.
118147
*/
119148
export function getClaudeShellEnvironment(): Record<string, string> {
120149
if (cachedShellEnv !== null) {
121150
return { ...cachedShellEnv }
122151
}
123152

124-
const shell = process.env.SHELL || "/bin/zsh"
153+
// Windows: use platform provider to build environment
154+
if (isWindows()) {
155+
console.log(
156+
"[claude-env] Windows detected, deriving PATH without shell invocation"
157+
)
158+
159+
// Use platform provider to build environment
160+
const env = platform.buildEnvironment()
161+
162+
// Strip sensitive keys
163+
stripSensitiveKeys(env)
164+
165+
console.log(
166+
`[claude-env] Built Windows environment with ${Object.keys(env).length} vars`
167+
)
168+
cachedShellEnv = env
169+
return { ...env }
170+
}
171+
172+
// macOS/Linux: spawn interactive login shell to get full environment
173+
const shell = getDefaultShell()
125174
const command = `echo -n "${DELIMITER}"; env; echo -n "${DELIMITER}"; exit`
126175

127176
try {
@@ -139,46 +188,23 @@ export function getClaudeShellEnvironment(): Record<string, string> {
139188
})
140189

141190
const env = parseEnvOutput(output)
142-
143-
// Strip keys that could interfere with Claude's auth resolution
144-
for (const key of STRIPPED_ENV_KEYS) {
145-
if (key in env) {
146-
console.log(`[claude-env] Stripped ${key} from shell environment`)
147-
delete env[key]
148-
}
149-
}
191+
stripSensitiveKeys(env)
150192

151193
console.log(
152-
`[claude-env] Loaded ${Object.keys(env).length} environment variables from shell`,
194+
`[claude-env] Loaded ${Object.keys(env).length} environment variables from shell`
153195
)
154196
cachedShellEnv = env
155197
return { ...env }
156198
} catch (error) {
157199
console.error("[claude-env] Failed to load shell environment:", error)
158200

159-
// Fallback: return minimal required env
160-
const home = os.homedir()
161-
const fallbackPath = [
162-
`${home}/.local/bin`,
163-
"/opt/homebrew/bin",
164-
"/usr/local/bin",
165-
"/usr/bin",
166-
"/bin",
167-
"/usr/sbin",
168-
"/sbin",
169-
].join(":")
170-
171-
const fallback: Record<string, string> = {
172-
HOME: home,
173-
USER: os.userInfo().username,
174-
PATH: fallbackPath,
175-
SHELL: process.env.SHELL || "/bin/zsh",
176-
TERM: "xterm-256color",
177-
}
201+
// Fallback: use platform provider
202+
const env = platform.buildEnvironment()
203+
stripSensitiveKeys(env)
178204

179-
console.log("[claude-env] Using fallback environment")
180-
cachedShellEnv = fallback
181-
return { ...fallback }
205+
console.log("[claude-env] Using fallback environment from platform provider")
206+
cachedShellEnv = env
207+
return { ...env }
182208
}
183209
}
184210

@@ -212,11 +238,17 @@ export function buildClaudeEnv(options?: {
212238
env.PATH = shellPath
213239
}
214240

215-
// 3. Ensure critical vars are present
216-
if (!env.HOME) env.HOME = os.homedir()
217-
if (!env.USER) env.USER = os.userInfo().username
218-
if (!env.SHELL) env.SHELL = "/bin/zsh"
241+
// 3. Ensure critical vars are present using platform provider
242+
const platformEnv = platform.buildEnvironment()
243+
if (!env.HOME) env.HOME = platformEnv.HOME
244+
if (!env.USER) env.USER = platformEnv.USER
219245
if (!env.TERM) env.TERM = "xterm-256color"
246+
if (!env.SHELL) env.SHELL = getDefaultShell()
247+
248+
// Windows-specific: ensure USERPROFILE is set
249+
if (isWindows() && !env.USERPROFILE) {
250+
env.USERPROFILE = os.homedir()
251+
}
220252

221253
// 4. Add custom overrides
222254
if (options?.ghToken) {
@@ -250,17 +282,17 @@ export function clearClaudeEnvCache(): void {
250282
*/
251283
export function logClaudeEnv(
252284
env: Record<string, string>,
253-
prefix: string = "",
285+
prefix: string = ""
254286
): void {
255287
console.log(`${prefix}[claude-env] HOME: ${env.HOME}`)
256288
console.log(`${prefix}[claude-env] USER: ${env.USER}`)
257289
console.log(
258-
`${prefix}[claude-env] PATH includes homebrew: ${env.PATH?.includes("/opt/homebrew")}`,
290+
`${prefix}[claude-env] PATH includes homebrew: ${env.PATH?.includes("/opt/homebrew")}`
259291
)
260292
console.log(
261-
`${prefix}[claude-env] PATH includes /usr/local/bin: ${env.PATH?.includes("/usr/local/bin")}`,
293+
`${prefix}[claude-env] PATH includes /usr/local/bin: ${env.PATH?.includes("/usr/local/bin")}`
262294
)
263295
console.log(
264-
`${prefix}[claude-env] ANTHROPIC_AUTH_TOKEN: ${env.ANTHROPIC_AUTH_TOKEN ? "set" : "not set"}`,
296+
`${prefix}[claude-env] ANTHROPIC_AUTH_TOKEN: ${env.ANTHROPIC_AUTH_TOKEN ? "set" : "not set"}`
265297
)
266298
}

src/main/lib/cli.ts

Lines changed: 23 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@
88

99
import { app } from "electron"
1010
import { join } from "path"
11-
import { existsSync, lstatSync, readlinkSync } from "fs"
11+
import { existsSync, lstatSync } from "fs"
12+
import { platform } from "./platform"
1213

1314
// Launch directory from CLI (e.g., `1code /path/to/project`)
1415
let launchDirectory: string | null = null
@@ -51,88 +52,42 @@ export function parseLaunchDirectory(): void {
5152
}
5253
}
5354

54-
// CLI command installation paths
55-
const CLI_INSTALL_PATH = "/usr/local/bin/1code"
56-
55+
/**
56+
* Get the CLI source path (where the CLI script is bundled)
57+
*/
5758
function getCliSourcePath(): string {
59+
const cliName = platform.getCliConfig().scriptName
5860
if (app.isPackaged) {
59-
return join(process.resourcesPath, "cli", "1code")
61+
return join(process.resourcesPath, "cli", cliName)
6062
}
61-
return join(__dirname, "..", "..", "resources", "cli", "1code")
63+
return join(__dirname, "..", "..", "resources", "cli", cliName)
6264
}
6365

6466
/**
6567
* Check if the CLI command is installed
6668
*/
6769
export function isCliInstalled(): boolean {
68-
try {
69-
if (!existsSync(CLI_INSTALL_PATH)) return false
70-
const stat = lstatSync(CLI_INSTALL_PATH)
71-
if (!stat.isSymbolicLink()) return false
72-
const target = readlinkSync(CLI_INSTALL_PATH)
73-
return target === getCliSourcePath()
74-
} catch {
75-
return false
76-
}
70+
return platform.isCliInstalled(getCliSourcePath())
7771
}
7872

7973
/**
80-
* Install the CLI command to /usr/local/bin/1code
81-
* Requires admin privileges on macOS
74+
* Install the CLI command
75+
* Platform-specific behavior is handled by the platform provider
8276
*/
83-
export async function installCli(): Promise<{ success: boolean; error?: string }> {
84-
const { exec } = await import("child_process")
85-
const { promisify } = await import("util")
86-
const execAsync = promisify(exec)
87-
88-
const sourcePath = getCliSourcePath()
89-
90-
if (!existsSync(sourcePath)) {
91-
return { success: false, error: "CLI script not found in app bundle" }
92-
}
93-
94-
try {
95-
// Remove existing if present
96-
if (existsSync(CLI_INSTALL_PATH)) {
97-
await execAsync(
98-
`osascript -e 'do shell script "rm -f ${CLI_INSTALL_PATH}" with administrator privileges'`,
99-
)
100-
}
101-
102-
// Create symlink with admin privileges
103-
await execAsync(
104-
`osascript -e 'do shell script "ln -s \\"${sourcePath}\\" ${CLI_INSTALL_PATH}" with administrator privileges'`,
105-
)
106-
107-
console.log("[CLI] Installed 1code command to", CLI_INSTALL_PATH)
108-
return { success: true }
109-
} catch (error: unknown) {
110-
const errorMessage = error instanceof Error ? error.message : "Installation failed"
111-
console.error("[CLI] Failed to install:", error)
112-
return { success: false, error: errorMessage }
113-
}
77+
export async function installCli(): Promise<{
78+
success: boolean
79+
error?: string
80+
}> {
81+
return platform.installCli(getCliSourcePath())
11482
}
11583

11684
/**
117-
* Uninstall the CLI command from /usr/local/bin/1code
118-
* Requires admin privileges on macOS
85+
* Uninstall the CLI command
86+
* Platform-specific behavior is handled by the platform provider
11987
*/
120-
export async function uninstallCli(): Promise<{ success: boolean; error?: string }> {
121-
const { exec } = await import("child_process")
122-
const { promisify } = await import("util")
123-
const execAsync = promisify(exec)
124-
125-
try {
126-
if (existsSync(CLI_INSTALL_PATH)) {
127-
await execAsync(
128-
`osascript -e 'do shell script "rm -f ${CLI_INSTALL_PATH}" with administrator privileges'`,
129-
)
130-
}
131-
console.log("[CLI] Uninstalled 1code command")
132-
return { success: true }
133-
} catch (error: unknown) {
134-
const errorMessage = error instanceof Error ? error.message : "Uninstallation failed"
135-
console.error("[CLI] Failed to uninstall:", error)
136-
return { success: false, error: errorMessage }
137-
}
88+
export async function uninstallCli(): Promise<{
89+
success: boolean
90+
error?: string
91+
}> {
92+
return platform.uninstallCli()
13893
}

0 commit comments

Comments
 (0)