-
Notifications
You must be signed in to change notification settings - Fork 0
runnign automate and finalzie #85
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
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
53ae749
runnign automate and finalzie
arul28 62daa9b
Update ci.yml
arul28 8652023
Update controlPlane.test.ts
arul28 dacf981
Fix valid PR #85 review comments across 17 files
arul28 e2818b8
Fix three failing tests from PR review fixes
arul28 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
40 changes: 40 additions & 0 deletions
40
apps/desktop/src/main/services/ai/claudeCodeExecutable.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| import { describe, expect, it } from "vitest"; | ||
| import { resolveClaudeCodeExecutable } from "./claudeCodeExecutable"; | ||
|
|
||
| describe("resolveClaudeCodeExecutable", () => { | ||
| it("prefers the explicit env override", () => { | ||
| expect( | ||
| resolveClaudeCodeExecutable({ | ||
| env: { | ||
| CLAUDE_CODE_EXECUTABLE_PATH: "/custom/bin/claude", | ||
| PATH: "/usr/bin:/bin", | ||
| }, | ||
| }), | ||
| ).toEqual({ | ||
| path: "/custom/bin/claude", | ||
| source: "env", | ||
| }); | ||
| }); | ||
|
|
||
| it("uses the detected Claude auth path before falling back to PATH lookup", () => { | ||
| expect( | ||
| resolveClaudeCodeExecutable({ | ||
| auth: [ | ||
| { | ||
| type: "cli-subscription", | ||
| cli: "claude", | ||
| path: "/opt/homebrew/bin/claude", | ||
| authenticated: true, | ||
| verified: true, | ||
| }, | ||
| ], | ||
| env: { | ||
| PATH: "/usr/bin:/bin", | ||
| }, | ||
| }), | ||
| ).toEqual({ | ||
| path: "/opt/homebrew/bin/claude", | ||
| source: "auth", | ||
| }); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| import fs from "node:fs"; | ||
| import os from "node:os"; | ||
| import path from "node:path"; | ||
| import type { DetectedAuth } from "./authDetector"; | ||
|
|
||
| export type ClaudeCodeExecutableResolution = { | ||
| path: string; | ||
| source: "env" | "auth" | "path" | "common-dir" | "fallback-command"; | ||
| }; | ||
|
|
||
| const HOME_DIR = os.homedir(); | ||
| const COMMON_BIN_DIRS = [ | ||
| "/opt/homebrew/bin", | ||
| "/opt/homebrew/sbin", | ||
| "/usr/local/bin", | ||
| "/usr/local/sbin", | ||
| "/usr/bin", | ||
| "/bin", | ||
| `${HOME_DIR}/.local/bin`, | ||
| `${HOME_DIR}/.nvm/current/bin`, | ||
| ].filter(Boolean); | ||
|
|
||
| function isExecutableFile(candidatePath: string): boolean { | ||
| try { | ||
| const stat = fs.statSync(candidatePath); | ||
| return stat.isFile() && (process.platform === "win32" || (stat.mode & 0o111) !== 0); | ||
| } catch { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| function resolveFromPathEntries(command: string, pathValue: string | undefined): string | null { | ||
| if (!pathValue) return null; | ||
| for (const entry of pathValue.split(path.delimiter)) { | ||
| const trimmed = entry.trim(); | ||
| if (!trimmed) continue; | ||
| const candidatePath = path.join(trimmed, command); | ||
| if (isExecutableFile(candidatePath)) { | ||
| return candidatePath; | ||
| } | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| function findClaudeAuthPath(auth?: DetectedAuth[]): string | null { | ||
| for (const entry of auth ?? []) { | ||
| if (entry.type !== "cli-subscription" || entry.cli !== "claude") continue; | ||
| const candidate = entry.path.trim(); | ||
| if (candidate) { | ||
| return candidate; | ||
| } | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| export function resolveClaudeCodeExecutable(args?: { | ||
| auth?: DetectedAuth[]; | ||
| env?: NodeJS.ProcessEnv; | ||
| }): ClaudeCodeExecutableResolution { | ||
| const env = args?.env ?? process.env; | ||
| const envPath = env.CLAUDE_CODE_EXECUTABLE_PATH?.trim(); | ||
| if (envPath) { | ||
| return { path: envPath, source: "env" }; | ||
| } | ||
|
|
||
| const authPath = findClaudeAuthPath(args?.auth); | ||
| if (authPath) { | ||
| return { path: authPath, source: "auth" }; | ||
| } | ||
|
|
||
| const pathResolved = resolveFromPathEntries("claude", env.PATH); | ||
| if (pathResolved) { | ||
| return { path: pathResolved, source: "path" }; | ||
| } | ||
|
|
||
| for (const binDir of COMMON_BIN_DIRS) { | ||
| const candidatePath = path.join(binDir, "claude"); | ||
| if (isExecutableFile(candidatePath)) { | ||
| return { path: candidatePath, source: "common-dir" }; | ||
| } | ||
| } | ||
|
|
||
| return { path: "claude", source: "fallback-command" }; | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
🧩 Analysis chain
🌐 Web query:
In Node.js/Electron on Windows, when manually resolving an executable from PATH, do you need to account for PATHEXT extensions like.exe/.cmdand thePATH/Pathenvironment-variable casing?💡 Result:
Yes, when manually resolving an executable from PATH in Node.js/Electron on Windows, you need to account for both PATHEXT extensions (like .exe, .cmd) and PATH/Path environment variable casing. Node.js child_process.spawn and spawnSync do not automatically handle PATHEXT; they only find .exe files by default and ignore PATHEXT for non-.exe files like .bat/.cmd. For manual resolution matching Windows shell behavior, implement PATH splitting, PATHEXT appending, and file existence checks. The PATH environment variable in process.env may appear as 'PATH' or 'Path' due to Windows case-insensitivity; check both or use case-insensitive lookup (e.g., find key where key.toUpperCase === 'PATH'). Use libraries like 'which' npm package, which properly handles PATH/PATHEXT on Windows. Electron uses the same Node.js child_process, so same rules apply.
Citations:
process.env.PATHcan be undefined on Windows npm/node-which#108🏁 Script executed:
Repository: arul28/ADE
Length of output: 111
🏁 Script executed:
# Read the file to understand the full context cat -n apps/desktop/src/main/services/ai/claudeCodeExecutable.tsRepository: arul28/ADE
Length of output: 2929
🏁 Script executed:
Repository: arul28/ADE
Length of output: 333
🏁 Script executed:
Repository: arul28/ADE
Length of output: 633
Windows executables require extension handling and case-insensitive PATH lookup.
On Windows, the
claudeexecutable is typically installed asclaude.exeorclaude.cmd. The current code only checks for the literal nameclaudewithout extensions, which will fail to find these installations on Windows. Additionally, the code only accessesenv.PATH, but on Windows this environment variable may be cased asPathdue to case-insensitivity.This affects both the PATH probe (line 67) and the
COMMON_BIN_DIRSprobe (lines 72-76), causing the code to fall through to the fallback"claude"which relies on the shell to resolve the executable.Suggested fix
🤖 Prompt for AI Agents