-
Notifications
You must be signed in to change notification settings - Fork 0
Fix Codex Desktop CLI discovery #24
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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,82 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { accessSync, constants } from "node:fs"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { homedir } from "node:os"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { delimiter, resolve } from "node:path"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const CODEX_DESKTOP_RELATIVE_PATHS = [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ["Applications", "ChatGPT.app", "Contents", "Resources", "codex"], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ["Applications", "Codex.app", "Contents", "Resources", "codex"], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export function resolveCodexCli(options = {}) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const env = options.env ?? process.env; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const requested = nonEmpty(options.requested) || nonEmpty(env.LOOP_IT_CODEX_BIN) || "codex"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (requested !== "codex") { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| bin: requested, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| source: options.requested ? "argument" : "environment", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+12
to
+18
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 Base the source on the normalized override. A truthy whitespace-only Proposed fix- const requested = nonEmpty(options.requested) || nonEmpty(env.LOOP_IT_CODEX_BIN) || "codex";
+ const requestedArgument = nonEmpty(options.requested);
+ const requestedEnvironment = nonEmpty(env.LOOP_IT_CODEX_BIN);
+ const requested = requestedArgument || requestedEnvironment || "codex";
...
- source: options.requested ? "argument" : "environment",
+ source: requestedArgument ? "argument" : "environment",📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (isCommandOnPath("codex", env)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| bin: "codex", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| source: "path", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (const candidate of desktopCandidates(env)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (isExecutable(candidate)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| bin: candidate, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| source: "desktop", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| bin: "codex", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| source: "missing", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function desktopCandidates(env) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const home = nonEmpty(env.HOME) || homedir(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const candidates = CODEX_DESKTOP_RELATIVE_PATHS.map((parts) => resolve(home, ...parts)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (process.platform === "darwin") { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| candidates.push( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "/Applications/ChatGPT.app/Contents/Resources/codex", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "/Applications/Codex.app/Contents/Resources/codex" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return [...new Set(candidates)]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+43
to
+55
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 Only consider desktop candidates on macOS. The home-relative candidates are built before the Darwin check, so Linux and Windows also treat Proposed fix function desktopCandidates(env) {
+ if (process.platform !== "darwin") {
+ return [];
+ }
+
const home = nonEmpty(env.HOME) || homedir();
const candidates = CODEX_DESKTOP_RELATIVE_PATHS.map((parts) => resolve(home, ...parts));
- if (process.platform === "darwin") {
- candidates.push(
- "/Applications/ChatGPT.app/Contents/Resources/codex",
- "/Applications/Codex.app/Contents/Resources/codex"
- );
- }
+ candidates.push(
+ "/Applications/ChatGPT.app/Contents/Resources/codex",
+ "/Applications/Codex.app/Contents/Resources/codex"
+ );
return [...new Set(candidates)];
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function isCommandOnPath(command, env) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const pathEntries = String(env.PATH ?? "") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .split(delimiter) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .map((entry) => entry.trim().replace(/^"|"$/g, "")) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .filter(Boolean); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const extensions = process.platform === "win32" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ? String(env.PATHEXT ?? ".EXE;.CMD;.BAT;.COM").split(delimiter) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| : [""]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return pathEntries.some((entry) => | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| extensions.some((extension) => isExecutable(resolve(entry, `${command}${extension}`))) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function isExecutable(path) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| accessSync(path, constants.X_OK); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return true; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function nonEmpty(value) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return typeof value === "string" && value.trim() ? value.trim() : ""; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Reject directories as executable candidates.
accessSync(path, constants.X_OK)can succeed for a traversable directory on POSIX. A directory namedcodexinPATHor a desktop candidate can therefore be selected, causing downstreamspawnSyncexecution to fail instead of trying another candidate.Proposed fix
Also applies to: 71-77
🤖 Prompt for AI Agents