From 06f00827c9e63cad2071ce047b473740350adca1 Mon Sep 17 00:00:00 2001 From: abose Date: Wed, 16 Sep 2026 19:32:32 +0530 Subject: [PATCH 1/3] fix(terminal): harden CLI launches and update xterm Preserve CLI installation directories on PATH for probes and terminal launches, including npm shims and standalone executables. Register CLI locator coverage in the existing Jasmine runner with Node test helpers. Keep immediate process exits from being overwritten by spawn completion, and add strings for the CLI exit banner and restart action. Pin xterm and its addons to the tested beta versions to fix animated glyph corruption and shared texture atlas invalidation. Document code style and test runner conventions for agents. Validation: Terminal integration 13/13; AI Chat CLI integration 18/18; CLI Locator suite 11/11; package pins and tarball integrity verified. --- AGENTS.md | 14 ++ CLAUDE.md | 7 + package-lock.json | 98 +++++----- package.json | 10 +- src-node/claude-code-agent.js | 2 +- src-node/cli-locator.js | 33 +++- src-node/test-connection.js | 1 + src-node/test/test-cli-locator.js | 168 ++++++++++++++++++ .../Terminal/TerminalInstance.js | 5 +- src/nls/root/strings.js | 2 + test/UnitTestSuite.js | 1 + test/spec/CLILocator-test.js | 126 +++++++++++++ 12 files changed, 415 insertions(+), 52 deletions(-) create mode 100644 AGENTS.md create mode 100644 src-node/test/test-cli-locator.js create mode 100644 test/spec/CLILocator-test.js diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000000..150a02e5e5 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,14 @@ +# Repository Instructions + +Follow [CLAUDE.md](CLAUDE.md) for repository workflows and requirements. + +## Code Style and Readability +- New code must match the surrounding file's formatting and conventions while following the repository's explicit style rules. +- Keep all imports and `require()` declarations at the top of the file (or the top of its AMD module factory). Do not use dynamic or asynchronous imports. +- Write clear, concise JSDoc for new or changed functions, documenting their purpose, parameters, and return values where relevant. Prioritize readability over verbosity; explain non-obvious behavior without repeating the code. + +## Tests +- Use the existing Jasmine runner and CI registration. Do not add standalone `node:test` suites or separate test commands unless explicitly requested. +- Before writing tests, inspect a nearby suite, its registration, and the relevant CI workflow. Core specs live in `test/spec/` and are registered in `test/UnitTestSuite.js`; Node-side coverage follows `test/spec/CLILocator-test.js` and `src-node/test/test-cli-locator.js`. +- Follow [CLAUDE.md — Writing Tests](CLAUDE.md#writing-tests) for categories, Node helpers, fixture isolation, and CI coverage. Keep individual cases visible as separate Jasmine `it()` results. +- Verify new suites in the connected `phoenix-test-runner-*` instance using `run_tests` and `get_test_results`. Confirm the expected spec count and category; a passing standalone script or a run with zero specs is not sufficient. diff --git a/CLAUDE.md b/CLAUDE.md index ddffdd6772..cb971c5446 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -58,6 +58,13 @@ Use `exec_js` to run JS in the Phoenix browser runtime. jQuery `$()` is global. When asked to "run the AI test suite" / "run the model tests" / "run EC-1 and UB-2": call `run_ai_test_suite` (phoenix-builder MCP) with `suite` (`quick` | `all` | a suite name), or `tests` for specific IDs, or `resumeRunId` to continue. It installs the fixture, opens a run record, and returns the briefing plus the test documents from `src/extensionsIntegrated/phoenix-pro/unit-tests/ai_model_tests/`. You are the runner and the judge — follow them exactly, deterministic checks first. After **every** test call `ai_test_progress` and tell the user one progress line. If the user says stop: `ai_test_progress({ runId, stop: true })`, then save. Finish with `save_ai_test_report`, then `compare_ai_test_reports({})`, and tell the user the report path, PASS/FAIL counts, any regressions, and the Observations section. ## Writing Tests +- Use the existing **Jasmine + AMD** framework (`describe`, `it`, `expect`). Do not introduce standalone `node:test` suites or separate runners unless explicitly requested; unregistered tests do not run in CI. +- Before writing a test, inspect a nearby suite, its registration, and the relevant `.github/workflows/` desktop or browser job. Register core specs from `test/spec/` in **`test/UnitTestSuite.js`**. Pro extension specs use `src/extensionsIntegrated/phoenix-pro/unittests.js`; shared `src-node` coverage belongs in the core suite. +- Give new suites an explicit supported category, such as `unit:CLI Locator`. Use `unit` for logic and Node subprocess tests that need no editor iframe or window focus; use `integration` for UI/editor behavior. Desktop Linux, Windows, and macOS CI jobs already run the `unit` category. +- For Node-side behavior, follow `test/spec/CLILocator-test.js` and `src-node/test/test-cli-locator.js`: keep assertions in separate Jasmine `it()` cases and invoke Node helpers with `execPeer()`. Place Node helpers in `src-node/test/` and load them through the existing `src-node/test-connection.js` bootstrap with top-level imports and a dedicated connector ID. Keep helper operations bounded; do not add arbitrary remote-code execution APIs. +- Isolate each test's environment, caches, files, and processes. Use temporary fixtures, bundled `process.execPath` for Node scripts, and `try/finally` cleanup. Do not depend on user-installed CLIs, account credentials, network access, or mutate the app's `process.env`. Use asynchronous child-process APIs so the shared Node event loop remains responsive. +- Gate Node-only suites on `Phoenix.isNativeApp`. For unsupported platform fixtures, guard their registration and explain why; **do not use Jasmine `pending()`/`xit()` as skips**, because Phoenix's reporter treats pending specs as failures. Distinguish simulated platform checks from native execution coverage and verify the expected spec count on each platform. +- Validate through the connected test runner: confirm the suite is discovered with the expected spec count and category, run it using MCP, and check `get_test_results` for completion and failures. A zero-spec run is not a pass. When adding shared test bootstrap code, also run the unit category to check that other suites still work. - **Never use `awaits(number)`** (fixed-time waits) in tests — they cause flaky failures. Always use `awaitsFor(condition)` to wait for a specific condition to become true. - Use `editor.*` APIs (e.g. `editor.document.getText()`, `editor.getCursorPos()`, `editor.setSelection()`) instead of accessing `editor._codeMirror` directly. - Tests should be independent — no shared mutable state between `it()` blocks. Use `FILE_CLOSE` with `{ _forceClose: true }` to clean up. diff --git a/package-lock.json b/package-lock.json index d3d90c5def..b4e819d156 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "phoenix", - "version": "5.2.0-0", + "version": "5.5.3-0", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "phoenix", - "version": "5.2.0-0", + "version": "5.5.3-0", "hasInstallScript": true, "dependencies": { "@bugsnag/js": "^7.18.0", @@ -18,11 +18,11 @@ "@pixelbrackets/gfm-stylesheet": "^1.1.0", "@prettier/plugin-php": "^0.22.2", "@uiw/file-icons": "^1.3.2", - "@xterm/addon-fit": "^0.11.0", - "@xterm/addon-search": "^0.16.0", - "@xterm/addon-web-links": "^0.12.0", - "@xterm/addon-webgl": "^0.19.0", - "@xterm/xterm": "^6.0.0", + "@xterm/addon-fit": "0.12.0-beta.301", + "@xterm/addon-search": "0.17.0-beta.301", + "@xterm/addon-web-links": "0.13.0-beta.301", + "@xterm/addon-webgl": "0.20.0-beta.300", + "@xterm/xterm": "6.1.0-beta.304", "bootstrap": "^5.1.3", "browser-mime": "^1.0.1", "codemirror": "^5.65.16", @@ -1328,33 +1328,45 @@ } }, "node_modules/@xterm/addon-fit": { - "version": "0.11.0", - "resolved": "https://registry.npmjs.org/@xterm/addon-fit/-/addon-fit-0.11.0.tgz", - "integrity": "sha512-jYcgT6xtVYhnhgxh3QgYDnnNMYTcf8ElbxxFzX0IZo+vabQqSPAjC3c1wJrKB5E19VwQei89QCiZZP86DCPF7g==", - "license": "MIT" + "version": "0.12.0-beta.301", + "resolved": "https://registry.npmjs.org/@xterm/addon-fit/-/addon-fit-0.12.0-beta.301.tgz", + "integrity": "sha512-MukRyJLAFQrW/++aM209jBZ8G9J/Ioe4SGS6c3ba87v+zYuX/I66JrDB7sVfykQ9t7DlT+c1MWl1jTlX2kyx1w==", + "license": "MIT", + "peerDependencies": { + "@xterm/xterm": "^6.1.0-beta.304" + } }, "node_modules/@xterm/addon-search": { - "version": "0.16.0", - "resolved": "https://registry.npmjs.org/@xterm/addon-search/-/addon-search-0.16.0.tgz", - "integrity": "sha512-9OeuBFu0/uZJPu+9AHKY6g/w0Czyb/Ut0A5t79I4ULoU4IfU5BEpPFVGQxP4zTTMdfZEYkVIRYbHBX1xWwjeSA==", - "license": "MIT" + "version": "0.17.0-beta.301", + "resolved": "https://registry.npmjs.org/@xterm/addon-search/-/addon-search-0.17.0-beta.301.tgz", + "integrity": "sha512-Dj8q+p2/5c4fcS4hag6QaXngxstnc4DdzREN/IhpGxz5a1JpW/3FnF9/JE1P1wvNLp9fApgGmZqBMW4xCF6HKg==", + "license": "MIT", + "peerDependencies": { + "@xterm/xterm": "^6.1.0-beta.304" + } }, "node_modules/@xterm/addon-web-links": { - "version": "0.12.0", - "resolved": "https://registry.npmjs.org/@xterm/addon-web-links/-/addon-web-links-0.12.0.tgz", - "integrity": "sha512-4Smom3RPyVp7ZMYOYDoC/9eGJJJqYhnPLGGqJ6wOBfB8VxPViJNSKdgRYb8NpaM6YSelEKbA2SStD7lGyqaobw==", - "license": "MIT" + "version": "0.13.0-beta.301", + "resolved": "https://registry.npmjs.org/@xterm/addon-web-links/-/addon-web-links-0.13.0-beta.301.tgz", + "integrity": "sha512-BpY297K+FGLsTQemnA6uOg6hk85DwVuUvWCqE9rIAcDxOOC7fP9GKAZTIfaEos/EZk2Ei23vD6w0P5Br6+RU7Q==", + "license": "MIT", + "peerDependencies": { + "@xterm/xterm": "^6.1.0-beta.304" + } }, "node_modules/@xterm/addon-webgl": { - "version": "0.19.0", - "resolved": "https://registry.npmjs.org/@xterm/addon-webgl/-/addon-webgl-0.19.0.tgz", - "integrity": "sha512-b3fMOsyLVuCeNJWxolACEUED0vm7qC0cy4wRvf3oURSzDTYVQiGPhTnhWZwIHdvC48Y+oLhvYXnY4XDXPoJo6A==", - "license": "MIT" + "version": "0.20.0-beta.300", + "resolved": "https://registry.npmjs.org/@xterm/addon-webgl/-/addon-webgl-0.20.0-beta.300.tgz", + "integrity": "sha512-BJQiOx8I+zz2woe+/E9nWprRRPevo9slPgyOd43E8o18aG307ccsyaTJoIbSu99uqfDoVLWDRLTy91ZXTFsdig==", + "license": "MIT", + "peerDependencies": { + "@xterm/xterm": "^6.1.0-beta.304" + } }, "node_modules/@xterm/xterm": { - "version": "6.0.0", - "resolved": "https://registry.npmjs.org/@xterm/xterm/-/xterm-6.0.0.tgz", - "integrity": "sha512-TQwDdQGtwwDt+2cgKDLn0IRaSxYu1tSUjgKarSDkUM0ZNiSRXFpjxEsvc/Zgc5kq5omJ+V0a8/kIM2WD3sMOYg==", + "version": "6.1.0-beta.304", + "resolved": "https://registry.npmjs.org/@xterm/xterm/-/xterm-6.1.0-beta.304.tgz", + "integrity": "sha512-Wq9d4qFYslYQBIan+riXotdyurtREc6v+HWdAwV6Y2JSDI9eJ1dkWm/fzYb2rCGgGnM3baiYuSK4Z6OLNKLzDw==", "license": "MIT", "workspaces": [ "addons/*" @@ -14235,29 +14247,33 @@ "dev": true }, "@xterm/addon-fit": { - "version": "0.11.0", - "resolved": "https://registry.npmjs.org/@xterm/addon-fit/-/addon-fit-0.11.0.tgz", - "integrity": "sha512-jYcgT6xtVYhnhgxh3QgYDnnNMYTcf8ElbxxFzX0IZo+vabQqSPAjC3c1wJrKB5E19VwQei89QCiZZP86DCPF7g==" + "version": "0.12.0-beta.301", + "resolved": "https://registry.npmjs.org/@xterm/addon-fit/-/addon-fit-0.12.0-beta.301.tgz", + "integrity": "sha512-MukRyJLAFQrW/++aM209jBZ8G9J/Ioe4SGS6c3ba87v+zYuX/I66JrDB7sVfykQ9t7DlT+c1MWl1jTlX2kyx1w==", + "requires": {} }, "@xterm/addon-search": { - "version": "0.16.0", - "resolved": "https://registry.npmjs.org/@xterm/addon-search/-/addon-search-0.16.0.tgz", - "integrity": "sha512-9OeuBFu0/uZJPu+9AHKY6g/w0Czyb/Ut0A5t79I4ULoU4IfU5BEpPFVGQxP4zTTMdfZEYkVIRYbHBX1xWwjeSA==" + "version": "0.17.0-beta.301", + "resolved": "https://registry.npmjs.org/@xterm/addon-search/-/addon-search-0.17.0-beta.301.tgz", + "integrity": "sha512-Dj8q+p2/5c4fcS4hag6QaXngxstnc4DdzREN/IhpGxz5a1JpW/3FnF9/JE1P1wvNLp9fApgGmZqBMW4xCF6HKg==", + "requires": {} }, "@xterm/addon-web-links": { - "version": "0.12.0", - "resolved": "https://registry.npmjs.org/@xterm/addon-web-links/-/addon-web-links-0.12.0.tgz", - "integrity": "sha512-4Smom3RPyVp7ZMYOYDoC/9eGJJJqYhnPLGGqJ6wOBfB8VxPViJNSKdgRYb8NpaM6YSelEKbA2SStD7lGyqaobw==" + "version": "0.13.0-beta.301", + "resolved": "https://registry.npmjs.org/@xterm/addon-web-links/-/addon-web-links-0.13.0-beta.301.tgz", + "integrity": "sha512-BpY297K+FGLsTQemnA6uOg6hk85DwVuUvWCqE9rIAcDxOOC7fP9GKAZTIfaEos/EZk2Ei23vD6w0P5Br6+RU7Q==", + "requires": {} }, "@xterm/addon-webgl": { - "version": "0.19.0", - "resolved": "https://registry.npmjs.org/@xterm/addon-webgl/-/addon-webgl-0.19.0.tgz", - "integrity": "sha512-b3fMOsyLVuCeNJWxolACEUED0vm7qC0cy4wRvf3oURSzDTYVQiGPhTnhWZwIHdvC48Y+oLhvYXnY4XDXPoJo6A==" + "version": "0.20.0-beta.300", + "resolved": "https://registry.npmjs.org/@xterm/addon-webgl/-/addon-webgl-0.20.0-beta.300.tgz", + "integrity": "sha512-BJQiOx8I+zz2woe+/E9nWprRRPevo9slPgyOd43E8o18aG307ccsyaTJoIbSu99uqfDoVLWDRLTy91ZXTFsdig==", + "requires": {} }, "@xterm/xterm": { - "version": "6.0.0", - "resolved": "https://registry.npmjs.org/@xterm/xterm/-/xterm-6.0.0.tgz", - "integrity": "sha512-TQwDdQGtwwDt+2cgKDLn0IRaSxYu1tSUjgKarSDkUM0ZNiSRXFpjxEsvc/Zgc5kq5omJ+V0a8/kIM2WD3sMOYg==" + "version": "6.1.0-beta.304", + "resolved": "https://registry.npmjs.org/@xterm/xterm/-/xterm-6.1.0-beta.304.tgz", + "integrity": "sha512-Wq9d4qFYslYQBIan+riXotdyurtREc6v+HWdAwV6Y2JSDI9eJ1dkWm/fzYb2rCGgGnM3baiYuSK4Z6OLNKLzDw==" }, "accepts": { "version": "1.3.7", diff --git a/package.json b/package.json index 84fdb94257..f60ce865ff 100644 --- a/package.json +++ b/package.json @@ -122,10 +122,10 @@ "tern": "^0.24.3", "tinycolor2": "^1.4.2", "underscore": "^1.13.4", - "@xterm/xterm": "^6.0.0", - "@xterm/addon-fit": "^0.11.0", - "@xterm/addon-search": "^0.16.0", - "@xterm/addon-web-links": "^0.12.0", - "@xterm/addon-webgl": "^0.19.0" + "@xterm/xterm": "6.1.0-beta.304", + "@xterm/addon-fit": "0.12.0-beta.301", + "@xterm/addon-search": "0.17.0-beta.301", + "@xterm/addon-web-links": "0.13.0-beta.301", + "@xterm/addon-webgl": "0.20.0-beta.300" } } \ No newline at end of file diff --git a/src-node/claude-code-agent.js b/src-node/claude-code-agent.js index 3fd48f7f55..ee816f2fb4 100644 --- a/src-node/claude-code-agent.js +++ b/src-node/claude-code-agent.js @@ -814,7 +814,7 @@ exports.getCliSpawnProfile = async function (params) { return Object.assign({}, result, { command: null, args: [] }); } const profile = CliLocator.getSpawnProfile(result.path); - return Object.assign({}, result, { command: profile.command, args: profile.args }); + return Object.assign({}, result, profile); }; /** diff --git a/src-node/cli-locator.js b/src-node/cli-locator.js index 8dd6c94e0f..da9933a231 100644 --- a/src-node/cli-locator.js +++ b/src-node/cli-locator.js @@ -291,6 +291,24 @@ function canAccess(p) { } } +/** + * Add the CLI's bin directory to PATH so sibling tools (node/npm) work even + * when a GUI launch omits the installation directory. Keep the bin symlink's + * directory; its target in node_modules does not contain those tools. + * @param {string} cliPath - Resolved CLI path + * @param {Object} env - Environment to inherit PATH from + * @return {Object} PATH override only; excludes secrets from terminal profiles + */ +function _cliPathEnv(cliPath, env) { + const pathKey = isWindows + ? Object.keys(env).sort().find(key => key.toLowerCase() === "path") || "Path" + : "PATH"; + const binDir = path.dirname(path.resolve(cliPath)); + const inheritedPath = env[pathKey]; + const entries = inheritedPath ? inheritedPath.split(path.delimiter) : []; + return { [pathKey]: _dedupe([binDir, ...entries]).join(path.delimiter) }; +} + /** * Spawn a CLI with argv and resolve to { stdout, stderr, status, error }. * Async so callers don't block the event loop while it runs — `claude auth @@ -311,7 +329,10 @@ function spawnCli(cliPath, args, opts) { return new Promise(function (resolve) { const isCmdShim = isWindows && /\.(cmd|bat)$/i.test(cliPath); const spawnCmd = isCmdShim ? `"${cliPath}"` : cliPath; - const spawnOpts = isCmdShim ? Object.assign({ shell: true }, opts) : opts; + const baseEnv = (opts && opts.env) || process.env; + const spawnOpts = Object.assign(isCmdShim ? { shell: true } : {}, opts, { + env: Object.assign({}, baseEnv, _cliPathEnv(cliPath, baseEnv)) + }); const encoding = (opts && opts.encoding) || "utf8"; const timeoutMs = (opts && opts.timeout) || 0; let child; @@ -613,13 +634,17 @@ async function validateCliPath(cliId, cliPath) { * `cmd.exe /c`. Passing the shim straight through as the PTY's shell fails * to spawn, so every terminal caller must resolve through here rather than * using the raw path. - * @return {{command: string, args: Array}} + * `env` carries the same PATH used for probes so child tools such as npm + * remain available in the terminal. Callers must forward it to the PTY. + * @param {string} cliPath - Resolved CLI path + * @return {{command: string, args: Array, env: Object}} */ function getSpawnProfile(cliPath) { + const env = _cliPathEnv(cliPath, process.env); if (isWindows && /\.(cmd|bat)$/i.test(cliPath || "")) { - return { command: process.env.COMSPEC || "cmd.exe", args: ["/c", cliPath] }; + return { command: process.env.COMSPEC || "cmd.exe", args: ["/c", cliPath], env }; } - return { command: cliPath, args: [] }; + return { command: cliPath, args: [], env }; } exports.CLI_IDS = CLI_IDS; diff --git a/src-node/test-connection.js b/src-node/test-connection.js index b79302ef0b..f6dfe2a0e9 100644 --- a/src-node/test-connection.js +++ b/src-node/test-connection.js @@ -1,4 +1,5 @@ const NodeConnector = require("./node-connector"); +require("./test/test-cli-locator"); const TEST_NODE_CONNECTOR_ID = "ph_test_connector"; const nodeConnector = NodeConnector.createNodeConnector(TEST_NODE_CONNECTOR_ID, exports); diff --git a/src-node/test/test-cli-locator.js b/src-node/test/test-cli-locator.js new file mode 100644 index 0000000000..912efccead --- /dev/null +++ b/src-node/test/test-cli-locator.js @@ -0,0 +1,168 @@ +/* + * Copyright (c) 2021 - present core.ai + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +/** Node helpers for the Jasmine CLI Locator suite; assertions live in test/spec. */ +const childProcess = require("child_process"); +const fs = require("fs"); +const os = require("os"); +const path = require("path"); +const vm = require("vm"); +const NodeConnector = require("../node-connector"); + +const PROCESS_TIMEOUT_MS = 3000; + +/** + * Load the real locator with isolated platform, environment, and cache state. + * @param {string} platform - Node platform name + * @param {Object} env - Simulated desktop environment + * @param {Function} [spawnImpl] - Optional probe process substitute + * @return {Object} Locator exports + */ +function _loadLocator(platform, env, spawnImpl) { + const exported = {}; + const dependencies = { + path: platform === "win32" ? path.win32 : path.posix, + fs, + child_process: spawnImpl ? Object.assign({}, childProcess, { spawn: spawnImpl }) : childProcess + }; + const source = fs.readFileSync(path.join(__dirname, "..", "cli-locator.js"), "utf8"); + vm.runInNewContext(source, { + exports: exported, + process: { platform, env }, + console: { log() {} }, + setTimeout, + clearTimeout, + require(name) { + return dependencies[name]; + } + }, { filename: "cli-locator.js" }); + return exported; +} + +/** + * Run a fixture without blocking the shared Node process. + * @param {string} command - Executable path + * @param {string[]} args - Command arguments + * @param {Object} env - Complete child environment + * @return {Promise} Serializable exit status, output, and spawn error + */ +function _runProcess(command, args, env) { + return new Promise(function (resolve) { + childProcess.execFile(command, args, { + env, encoding: "utf8", timeout: PROCESS_TIMEOUT_MS, killSignal: "SIGKILL" + }, function (error, stdout, stderr) { + resolve({ + status: error ? (typeof error.code === "number" ? error.code : null) : 0, + errorCode: error && typeof error.code === "string" ? error.code : null, + stdout, + stderr + }); + }); + }); +} + +/** + * Return a launch profile and the input environment after the locator runs. + * @param {Object} params - Simulated platform, env, and cliPath + * @return {Promise} Profile and environment for Jasmine assertions + */ +async function getSpawnProfile({platform, env, cliPath}) { + const locator = _loadLocator(platform, env); + return { profile: locator.getSpawnProfile(cliPath), env }; +} + +/** + * Capture Windows probe options while running a harmless bundled-Node child. + * @return {Promise} Probe result, captured spawn options, and terminal profile + */ +async function probeWindowsShim() { + const env = { Path: "C:\\Windows;C:\\nodejs" }; + let captured; + const locator = _loadLocator("win32", env, function (command, args, options) { + captured = { command, args, options }; + return childProcess.spawn(process.execPath, ["-e", "console.log('codex-cli 1.0.0');"]); + }); + const cliPath = "C:\\npm prefix\\codex.cmd"; + const result = await locator.spawnCli(cliPath, ["--version"], { timeout: PROCESS_TIMEOUT_MS }); + return { result, captured, profile: locator.getSpawnProfile(cliPath), env }; +} + +/** + * Exercise an npm-style symlink and updater with no installation directory on PATH. + * Uses bundled Node and a fake npm; never installs packages or contacts a server. + * @return {Promise} Before/after process results and version validation + */ +async function runNpmFixture() { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "phoenix-cli-env-")); + try { + const binDir = path.join(root, "node prefix with spaces", "bin"); + const packageDir = path.join(root, "node_modules", "@openai", "codex", "bin"); + fs.mkdirSync(binDir, { recursive: true }); + fs.mkdirSync(packageDir, { recursive: true }); + fs.symlinkSync(process.execPath, path.join(binDir, "node")); + fs.writeFileSync(path.join(binDir, "npm"), "#!/usr/bin/env node\n" + + "console.log('UPDATER_OK ' + process.argv.slice(2).join(' '));\n", { mode: 0o755 }); + const cliScript = path.join(packageDir, "codex.js"); + fs.writeFileSync(cliScript, [ + "#!/usr/bin/env node", + "const childProcess = require('child_process');", + "if (process.argv.includes('--version')) {", + " console.log('codex-cli 1.0.0');", + "} else {", + " const result = childProcess.spawnSync('npm', ['install', '-g', '@openai/codex'],", + " { encoding: 'utf8' });", + " if (result.error) { console.error(result.error.code); process.exit(1); }", + " process.stdout.write(result.stdout);", + " process.exitCode = result.status;", + "}", + "" + ].join("\n"), { mode: 0o755 }); + const cliPath = path.join(binDir, "codex"); + fs.symlinkSync(cliScript, cliPath); + const env = { PATH: path.join(root, "empty-path") }; + const originalPath = env.PATH; + const locator = _loadLocator(process.platform, env); + + // Start the script by absolute Node path to reproduce the updater's + // missing-npm failure independently of its own node shebang. + const before = await _runProcess(process.execPath, [cliPath], env); + const validated = await locator.validateCliPath("codex", cliPath); + const probe = await locator.spawnCli(cliPath, ["--version"], { + env, encoding: "utf8", timeout: PROCESS_TIMEOUT_MS + }); + const profile = locator.getSpawnProfile(cliPath); + const after = await _runProcess(profile.command, profile.args, Object.assign({}, env, profile.env)); + return { before, validated, probe, after, originalPath, env }; + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } +} + +/** + * Run a standalone fixture with no Node/npm dependency on PATH. + * @return {Promise} Version validation and process result + */ +async function runStandaloneFixture() { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "phoenix-standalone-cli-")); + try { + const cliPath = path.join(root, "codex"); + fs.writeFileSync(cliPath, '#!/bin/sh\nif [ "$1" = "--version" ]; then\n' + + " printf 'codex-cli 1.0.0\\n'\nelse\n printf 'STANDALONE_OK\\n'\nfi\n", { mode: 0o755 }); + const env = { PATH: path.join(root, "empty-path") }; + const locator = _loadLocator(process.platform, env); + const validated = await locator.validateCliPath("codex", cliPath); + const profile = locator.getSpawnProfile(cliPath); + const result = await _runProcess(profile.command, profile.args, Object.assign({}, env, profile.env)); + return { validated, result }; + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } +} + +exports.getSpawnProfile = getSpawnProfile; +exports.probeWindowsShim = probeWindowsShim; +exports.runNpmFixture = runNpmFixture; +exports.runStandaloneFixture = runStandaloneFixture; +NodeConnector.createNodeConnector("ph_test_cli_locator", exports); diff --git a/src/extensionsIntegrated/Terminal/TerminalInstance.js b/src/extensionsIntegrated/Terminal/TerminalInstance.js index 2d6539b765..6807e68c06 100644 --- a/src/extensionsIntegrated/Terminal/TerminalInstance.js +++ b/src/extensionsIntegrated/Terminal/TerminalInstance.js @@ -105,6 +105,7 @@ define(function (require, exports, module) { this.$container = null; this._resizeTimeout = null; this._disposed = false; + this._processExited = false; this._webglAddon = null; this._lastDpr = null; @@ -214,7 +215,8 @@ define(function (require, exports, module) { env: env || undefined }); this.pid = result.pid; - this.isAlive = true; + // A short-lived process can exit before the createTerminal reply arrives. + this.isAlive = !this._processExited && !this._disposed; } catch (err) { console.error("Terminal: Failed to spawn PTY:", err); this.terminal.write("\r\n\x1b[31mFailed to start terminal: " + err.message + "\x1b[0m\r\n"); @@ -239,6 +241,7 @@ define(function (require, exports, module) { */ TerminalInstance.prototype._onTerminalExit = function (_event, eventData) { if (eventData.id === this.id) { + this._processExited = true; this.isAlive = false; if (this.terminal) { this.terminal.write("\r\n\x1b[90m[Process exited with code " + eventData.exitCode + "]\x1b[0m\r\n"); diff --git a/src/nls/root/strings.js b/src/nls/root/strings.js index 9d8815ef01..e22cc483c0 100644 --- a/src/nls/root/strings.js +++ b/src/nls/root/strings.js @@ -2886,6 +2886,8 @@ define({ "AI_CHAT_CODEX_STALE_PROJECT_MSG": "This Codex CLI session is still running in {0} — the open project is now {1}.", "AI_CHAT_CLI_STALE_PROJECT_SWITCH_BTN": "Switch to “{0}”", "AI_CHAT_CLI_STALE_PROJECT_STAY_BTN": "Stay on “{0}”", + "AI_CHAT_CLI_EXITED_MSG": "{0} has exited.", + "AI_CHAT_CLI_RESTART_BTN": "Restart", "AI_CHAT_SETTINGS_TITLE": "AI Settings", "AI_SETTINGS_API_KEY": "API Key", "AI_SETTINGS_BASE_URL": "Base URL", diff --git a/test/UnitTestSuite.js b/test/UnitTestSuite.js index b7a159286a..703b2f10c0 100644 --- a/test/UnitTestSuite.js +++ b/test/UnitTestSuite.js @@ -154,6 +154,7 @@ define(function (require, exports, module) { require("spec/Terminal-integ-test"); // Node Tests require("spec/NodeConnection-test"); + require("spec/CLILocator-test"); // pro test suite optional components require("./pro-test-suite"); // todo TEST_MODERN diff --git a/test/spec/CLILocator-test.js b/test/spec/CLILocator-test.js new file mode 100644 index 0000000000..fa5d07aa5a --- /dev/null +++ b/test/spec/CLILocator-test.js @@ -0,0 +1,126 @@ +/* + * Copyright (c) 2021 - present core.ai + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +/*global describe, it, expect, beforeAll, awaitsFor */ + +define(function (require, exports, module) { + const NodeConnector = require("NodeConnector"); + + // The desktop unit jobs exercise Node helpers; browser jobs have no Node runtime. + if (!Phoenix.isNativeApp) { + return; + } + + describe("unit:CLI Locator", function () { + let nodeConnector; + + beforeAll(async function () { + await awaitsFor(NodeConnector.isNodeReady, "Node runtime to be ready"); + nodeConnector = NodeConnector.createNodeConnector("ph_test_cli_locator", exports); + }); + + ["linux", "darwin"].forEach(function (platform) { + it(platform + ": should keep the discovered nvm CLI's node/npm directory on PATH", async function () { + const env = { PATH: "/usr/bin:/bin", SECRET: "must stay on the node side" }; + const cliPath = "/home/test/.nvm/versions/node/v24.1.0/bin/codex"; + const result = await nodeConnector.execPeer("getSpawnProfile", { platform, env, cliPath }); + expect(result.profile.command).toBe(cliPath); + expect(result.profile.args).toEqual([]); + expect(result.profile.env.PATH).toBe( + "/home/test/.nvm/versions/node/v24.1.0/bin:/usr/bin:/bin"); + expect(Object.keys(result.profile.env)).toEqual(["PATH"]); + expect(result.env).toEqual(env); + }); + }); + + it("macOS: should make Homebrew bin available to the updater", async function () { + const result = await nodeConnector.execPeer("getSpawnProfile", { + platform: "darwin", + env: { PATH: "/usr/bin:/bin:/usr/sbin:/sbin" }, + cliPath: "/opt/homebrew/bin/codex" + }); + expect(result.profile.env.PATH).toBe("/opt/homebrew/bin:/usr/bin:/bin:/usr/sbin:/sbin"); + }); + + it("should prioritize the selected installation without duplicating its directory", async function () { + const result = await nodeConnector.execPeer("getSpawnProfile", { + platform: "linux", + env: { PATH: "/usr/bin:/opt/node/bin:/bin" }, + cliPath: "/opt/node/bin/codex" + }); + expect(result.profile.env.PATH).toBe("/opt/node/bin:/usr/bin:/bin"); + }); + + it("should not add the current directory when PATH is absent", async function () { + const result = await nodeConnector.execPeer("getSpawnProfile", { + platform: "linux", env: {}, cliPath: "/opt/node/bin/codex" + }); + expect(result.profile.env.PATH).toBe("/opt/node/bin"); + }); + + ["Path", "PATH"].forEach(function (pathKey) { + it("Windows: should retain cmd.exe and inherited " + pathKey + " casing", async function () { + const env = { + [pathKey]: "C:\\Windows\\System32;C:\\Program Files\\nodejs", + COMSPEC: "C:\\Windows\\System32\\cmd.exe" + }; + const cliPath = "C:\\Users\\Test User\\AppData\\Roaming\\npm\\codex.cmd"; + const result = await nodeConnector.execPeer("getSpawnProfile", { + platform: "win32", env, cliPath + }); + expect(result.profile.command).toBe(env.COMSPEC); + expect(result.profile.args).toEqual(["/c", cliPath]); + expect(result.profile.env[pathKey]).toBe( + "C:\\Users\\Test User\\AppData\\Roaming\\npm;" + env[pathKey]); + expect(Object.keys(result.profile.env)).toEqual([pathKey]); + expect(result.env).toEqual(env); + }); + }); + + it("Windows: should launch native executables directly and deduplicate PATH without case", async function () { + const cliPath = "c:\\tools\\codex.exe"; + const result = await nodeConnector.execPeer("getSpawnProfile", { + platform: "win32", env: { Path: "C:\\Windows;C:\\Tools;C:\\nodejs" }, cliPath + }); + expect(result.profile.command).toBe(cliPath); + expect(result.profile.args).toEqual([]); + expect(result.profile.env.Path).toBe("c:\\tools;C:\\Windows;C:\\nodejs"); + }); + + it("Windows: should give version probes the same PATH as terminal launches", async function () { + const result = await nodeConnector.execPeer("probeWindowsShim"); + expect(result.result.status).withContext(result.result.stderr).toBe(0); + expect(result.result.stdout.trim()).toBe("codex-cli 1.0.0"); + expect(result.captured.command).toBe('"C:\\npm prefix\\codex.cmd"'); + expect(result.captured.args).toEqual(["--version"]); + expect(result.captured.options.shell).toBeTrue(); + expect(result.captured.options.env.Path).toBe(result.profile.env.Path); + expect(result.env.Path).toBe("C:\\Windows;C:\\nodejs"); + }); + + // These fixtures need POSIX symlinks/shebangs. Register them only there: + // Phoenix's reporter treats Jasmine pending specs as failures. + if (Phoenix.platform !== "win") { + it("should run an npm-style CLI and updater with an otherwise empty desktop PATH", async function () { + const result = await nodeConnector.execPeer("runNpmFixture"); + expect(result.before.status).withContext(JSON.stringify(result.before)).toBe(1); + expect(result.before.stderr).toContain("ENOENT"); + expect(result.validated.ok).withContext(JSON.stringify(result.validated)).toBeTrue(); + expect(result.probe.status).withContext(result.probe.stderr).toBe(0); + expect(result.probe.stdout.trim()).toBe("codex-cli 1.0.0"); + expect(result.after.status).withContext(JSON.stringify(result.after)).toBe(0); + expect(result.after.stdout.trim()).toBe("UPDATER_OK install -g @openai/codex"); + expect(result.env.PATH).toBe(result.originalPath); + }, 15000); + + it("should launch a standalone CLI without node or npm on PATH", async function () { + const result = await nodeConnector.execPeer("runStandaloneFixture"); + expect(result.validated.ok).withContext(JSON.stringify(result.validated)).toBeTrue(); + expect(result.result.status).withContext(JSON.stringify(result.result)).toBe(0); + expect(result.result.stdout.trim()).toBe("STANDALONE_OK"); + }, 15000); + } + }); +}); From c0d93486b417cd961376b1f85b0e073a21bf2e96 Mon Sep 17 00:00:00 2001 From: abose Date: Wed, 16 Sep 2026 19:58:56 +0530 Subject: [PATCH 2/3] fix(build): preserve CSS BOMs across copy steps --- gulpfile.js/index.js | 30 ++++++++++++++++-------------- gulpfile.js/thirdparty-lib-copy.js | 15 +++++++++++++-- 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/gulpfile.js/index.js b/gulpfile.js/index.js index 14ca1b12c0..ef182a36e3 100644 --- a/gulpfile.js/index.js +++ b/gulpfile.js/index.js @@ -39,6 +39,8 @@ const rename = require("gulp-rename"); const execSync = require('child_process').execSync; const terser = require('terser'); +const copyOptions = copyThirdPartyLibs.copyOptions; + function cleanDist() { return del(['dist', 'dist-test']); } @@ -136,7 +138,7 @@ function _deletePhoenixProSourceFolder() { * @returns {*} */ function makeDistAll() { - return src(['src/**/*', 'src/.*/*.*']) + return src(['src/**/*', 'src/.*/*.*'], copyOptions) .pipe(dest('dist')); } @@ -170,7 +172,7 @@ function makeJSDist() { // we had to do this as prettier is non minifiable function makeJSPrettierDist() { - return src(["src/thirdparty/prettier/**/*"]) + return src(["src/thirdparty/prettier/**/*"], copyOptions) .pipe(dest('dist/thirdparty/prettier')); } @@ -182,12 +184,12 @@ function makeNonMinifyDist() { "src/LiveDevelopment/BrowserScripts/RemoteFunctions.js", "src/extensionsIntegrated/phoenix-pro/onboarding/**/*", "src/extensionsIntegrated/phoenix-pro/unit-tests/**/*", - "src/mdViewer/**/*"], {base: 'src'}) + "src/mdViewer/**/*"], {...copyOptions, base: 'src'}) .pipe(dest('dist')); } function makeDistNonJS() { - return src(['src/**/*', 'src/.*/*.*', '!src/**/*.js']) + return src(['src/**/*', 'src/.*/*.*', '!src/**/*.js'], copyOptions) .pipe(dest('dist')); } @@ -230,40 +232,40 @@ function zipTestFiles() { 'test/**', 'test/**/.*', '!test/thirdparty/**', - '!test/test_folders.zip']) + '!test/test_folders.zip'], copyOptions) .pipe(zip('test_folders.zip')) .pipe(dest('test/')); } function zipDefaultProjectFiles() { - return src(['src/assets/default-project/en/**']) + return src(['src/assets/default-project/en/**'], copyOptions) .pipe(zip('en.zip')) .pipe(dest('src/assets/default-project/')); } // sample projects function zipSampleProjectBootstrapBlog() { - return src(['src/assets/sample-projects/bootstrap-blog/**']) + return src(['src/assets/sample-projects/bootstrap-blog/**'], copyOptions) .pipe(zip('bootstrap-blog.zip')) .pipe(dest('src/assets/sample-projects/')); } function zipSampleProjectExplore() { - return src(['src/assets/sample-projects/explore/**']) + return src(['src/assets/sample-projects/explore/**'], copyOptions) .pipe(zip('explore.zip')) .pipe(dest('src/assets/sample-projects/')); } function zipSampleProjectHTML5() { - return src(['src/assets/sample-projects/HTML5/**']) + return src(['src/assets/sample-projects/HTML5/**'], copyOptions) .pipe(zip('HTML5.zip')) .pipe(dest('src/assets/sample-projects/')); } function zipSampleProjectDashboard() { - return src(['src/assets/sample-projects/dashboard/**']) + return src(['src/assets/sample-projects/dashboard/**'], copyOptions) .pipe(zip('dashboard.zip')) .pipe(dest('src/assets/sample-projects/')); } function zipSampleProjectHomePages() { - return src(['src/assets/sample-projects/home-pages/**']) + return src(['src/assets/sample-projects/home-pages/**'], copyOptions) .pipe(zip('home-pages.zip')) .pipe(dest('src/assets/sample-projects/')); } @@ -991,12 +993,12 @@ function createDistCacheManifestDev() { } function copyDistToDistTestFolder() { - return src('dist/**/*') + return src('dist/**/*', copyOptions) .pipe(dest('dist-test/src')); } function copyTestToDistTestFolder() { - return src('test/**/*') + return src('test/**/*', copyOptions) .pipe(dest('dist-test/test')); } @@ -1094,7 +1096,7 @@ function _patchMinifiedCSSInDistIndex() { return new Promise((resolve)=>{ let content = fs.readFileSync("dist/index.html", "utf8"); if(!content.includes(``)){ - throw new Error(`Could not locate string in file dist/index.html`) + throw new Error(`Could not locate string in file dist/index.html`); } content = content.replace( ``, diff --git a/gulpfile.js/thirdparty-lib-copy.js b/gulpfile.js/thirdparty-lib-copy.js index cc064520a2..cb1ce162a8 100644 --- a/gulpfile.js/thirdparty-lib-copy.js +++ b/gulpfile.js/thirdparty-lib-copy.js @@ -27,6 +27,16 @@ const path = require('path'); // removed require('merge-stream') node module. it gives wired glob behavior and some files goes missing const rename = require("gulp-rename"); +const copyOptions = { + /** + * Preserve CSS encoding markers; keep Gulp's BOM stripping for other file types. + * @param {Object} file Vinyl file being read. + * @returns {boolean} Whether to remove the file's BOM. + */ + removeBOM(file) { + return file.extname.toLowerCase() !== ".css"; + } +}; // individual third party copy function copyLicence(filePath, name) { @@ -38,7 +48,7 @@ function copyLicence(filePath, name) { function renameFile(filePath, newName, destPath) { console.log(`Renaming file ${filePath} to ${newName}`); - return src(filePath) + return src(filePath, copyOptions) .pipe(rename(newName)) .pipe(dest(destPath)); } @@ -65,7 +75,7 @@ function downloadFile(url, outputPath) { function copyFiles(srcPathList, dstPath) { console.log(`Copying files ${dstPath}`); - return src(srcPathList) + return src(srcPathList, copyOptions) .pipe(dest(dstPath)); } @@ -304,5 +314,6 @@ function _patchTernLib() { }); } +exports.copyOptions = copyOptions; exports.copyAll = series(copyThirdPartyLibs, _patchAcornLib, _patchTernLib); exports.copyAllDebug = series(copyThirdPartyLibs, copyThirdPartyDebugLibs, _patchAcornLib, _patchTernLib); From 81400e981a63dcfb2191a3e4c6b09d2e7108a652 Mon Sep 17 00:00:00 2001 From: abose Date: Wed, 16 Sep 2026 21:32:49 +0530 Subject: [PATCH 3/3] build: update pro deps --- tracking-repos.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tracking-repos.json b/tracking-repos.json index a808135a11..d2a173a22c 100644 --- a/tracking-repos.json +++ b/tracking-repos.json @@ -1,5 +1,5 @@ { "phoenixPro": { - "commitID": "adf1a4ec83cc3384d5e5c3677b4ada9191523222" + "commitID": "ac067fcd943e145b0ba3f92782678f661371b9a1" } }