From 95a9285f02a096fa725f22b70e30e4adc368cbb9 Mon Sep 17 00:00:00 2001 From: abose Date: Wed, 30 Sep 2026 17:40:03 +0530 Subject: [PATCH 1/8] fix: stabilize desktop tests and asynchronous editor updates --- .codex/config.toml | 3 + serve-proxy.js | 22 +- src-node/index.js | 57 ++- src/JSUtils/ScopeManager.js | 36 +- src/editor/MediaViewer.js | 9 +- .../DocCommentHints/integration-tests.js | 7 +- .../default/HTMLCodeHints/integ-tests.js | 47 ++- src/extensions/default/QuickView/unittests.js | 12 +- .../default/TypeScriptSupport/unittests.js | 33 +- src/language/CodeInspection.js | 39 +- src/node-loader.js | 32 ++ src/project/FileSyncManager.js | 52 ++- src/project/ProjectModel.js | 6 +- test/UnitTestSuite.js | 1 + test/spec/CodeInspection-fix-integ-test.js | 62 ++- test/spec/Document-integ-test.js | 369 +++++++++++++++++- test/spec/Extn-ESLint-integ-test.js | 27 +- test/spec/Extn-Git-integ-test.js | 38 +- test/spec/LiveDevelopmentMultiBrowser-test.js | 7 +- test/spec/ProjectModel-test.js | 18 + test/spec/ScopeManager-integ-test.js | 99 +++++ test/spec/SpecRunnerUtils.js | 38 ++ test/spec/md-editor-integ-test.js | 27 +- 23 files changed, 923 insertions(+), 118 deletions(-) create mode 100644 test/spec/ScopeManager-integ-test.js diff --git a/.codex/config.toml b/.codex/config.toml index 4192b15039..16acc3faef 100644 --- a/.codex/config.toml +++ b/.codex/config.toml @@ -27,3 +27,6 @@ import(pathToFileURL(path.join(root, entry)).href).catch((error) => { env_vars = ["PHOENIX_DESKTOP_PATH", "PHOENIX_MCP_WS_PORT"] # Live-preview connection and execution can take longer than the default 60s. tool_timeout_sec = 120 + +[mcp_servers.phoenix-builder.tools.get_phoenix_status] +approval_mode = "approve" diff --git a/serve-proxy.js b/serve-proxy.js index d7a1819902..2e2dbc684d 100644 --- a/serve-proxy.js +++ b/serve-proxy.js @@ -309,8 +309,26 @@ const server = http.createServer((req, res) => { } } - // Serve static files - let filePath = path.join(config.root, parsedUrl.pathname); + // Serve static files. url.parse leaves the pathname percent encoded, so a + // directory or file whose name contains a space was looked up on disk as + // "sub%20dir" and answered 404 even though it was right there. Decode before + // touching the filesystem - the traversal check below still runs on the + // result, which is what stops an encoded "%2e%2e" from buying anything. + let decodedPathname; + try { + decodedPathname = decodeURIComponent(parsedUrl.pathname); + } catch (e) { + // A malformed escape is a bad request, not a missing file. + res.writeHead(400, { 'Content-Type': 'text/plain' }); + res.end('Bad Request'); + return; + } + if (decodedPathname.indexOf('\0') !== -1) { + res.writeHead(400, { 'Content-Type': 'text/plain' }); + res.end('Bad Request'); + return; + } + let filePath = path.join(config.root, decodedPathname); // Security: prevent directory traversal const normalizedPath = path.normalize(filePath); diff --git a/src-node/index.js b/src-node/index.js index 5cde37499f..795cd055b3 100644 --- a/src-node/index.js +++ b/src-node/index.js @@ -169,17 +169,62 @@ rl.on('close', () => { process.exit(1); }); +// Opt-in watchdog for test processes; disabled by default. node-loader.js arms +// it for Phoenix.isTestWindow, and SpecRunnerUtils refreshes the runner and its +// test window. Normal editor sessions never send setIdleExit. +// +// Tauri owns this process and holds stdin open after the spawning page goes away. +// A terminate command sent during beforeunload can be lost, leaving Node and its +// LSP servers running. Test runs accept timeout-based cleanup for these orphans. +// Keep this disabled in normal editor sessions: sleep, page suspension or long +// pauses can stop heartbeats even though the page still exists. Expiry can then +// trigger a Node crash dialog after wake. Earlier heartbeat/socket orphan checks +// were removed for sleep-related crashes in a1e660cb5 and ba5800d93. +let _idleExitMs = 0; +let _idleExitTimer = null; + +function _shutdown(reason) { + lmdb.dumpDBToFileAndCloseDB() + .catch(console.error) + .finally(() => { + console.log(reason); + process.exit(0); + }); +} + +function _refreshIdleExit() { + if (!_idleExitMs) { + return; + } + if (_idleExitTimer) { + clearTimeout(_idleExitTimer); + } + _idleExitTimer = setTimeout(() => { + _shutdown(`No command for ${_idleExitMs}ms, the page that spawned us is gone.`); + }, _idleExitMs); + // never let this timer alone hold the process up + if (_idleExitTimer.unref) { + _idleExitTimer.unref(); + } +} + function processCommand(line) { try{ let jsonCmd = JSON.parse(line); + // any command at all is proof the page is still there + _refreshIdleExit(); switch (jsonCmd.commandCode) { + case "setIdleExit": + _idleExitMs = Number(jsonCmd.commandData) || 0; + if (!_idleExitMs && _idleExitTimer) { + clearTimeout(_idleExitTimer); + _idleExitTimer = null; + } + _refreshIdleExit(); + _sendResponse(_idleExitMs, jsonCmd.commandID); + return; case "terminate": - lmdb.dumpDBToFileAndCloseDB() - .catch(console.error) - .finally(()=>{ - console.log("Node terminated by phcode."); - process.exit(0); - }); + _shutdown("Node terminated by phcode."); return; case "ping": _sendResponse("pong", jsonCmd.commandID); return; case "setDebugMode": diff --git a/src/JSUtils/ScopeManager.js b/src/JSUtils/ScopeManager.js index ab6944cac1..c7ecfa1cbd 100644 --- a/src/JSUtils/ScopeManager.js +++ b/src/JSUtils/ScopeManager.js @@ -110,20 +110,22 @@ define(function (require, exports, module) { /** * Init preferences from a file in the project root or builtin - * defaults if no file is found; + * defaults if no file is found. Ignore callbacks from a previous project. * @private * @param {string=} projectRootPath - new project root path. Only needed * for unit tests. */ function initPreferences(projectRootPath) { - // Reject the old preferences if they have not completed. - if (deferredPreferences && deferredPreferences.state() === "pending") { - deferredPreferences.reject(); + const previousRequest = deferredPreferences; + const request = $.Deferred(); + deferredPreferences = request; + // Cancelling the old request can release queued editor changes. Those + // changes must already see the new project's preference request. + if (previousRequest && previousRequest.state() === "pending") { + previousRequest.reject(); } - - deferredPreferences = $.Deferred(); - var pr = ProjectManager.getProjectRoot(); + const pr = ProjectManager.getProjectRoot(); // Open preferences relative to the project root // Normally there is a project root, but for unit tests we need to @@ -133,6 +135,7 @@ define(function (require, exports, module) { } else if (!projectRootPath) { console.log("initPreferences: projectRootPath has no value. Using Defaults."); preferences = new Preferences(); + request.resolve(); return; } @@ -140,8 +143,14 @@ define(function (require, exports, module) { preferences = new Preferences(); FileSystem.resolve(path, function (err, file) { + if (deferredPreferences !== request) { + return; + } if (!err) { FileUtils.readAsText(file).done(function (text) { + if (deferredPreferences !== request) { + return; + } var configObj = null; try { configObj = JSON.parse(text); @@ -154,13 +163,16 @@ define(function (require, exports, module) { } } preferences = new Preferences(configObj); - deferredPreferences.resolve(); + request.resolve(); }).fail(function (error) { + if (deferredPreferences !== request) { + return; + } preferences = new Preferences(); - deferredPreferences.resolve(); + request.resolve(); }); } else { - deferredPreferences.resolve(); + request.resolve(); } }); } @@ -1299,6 +1311,10 @@ define(function (require, exports, module) { }); }); }); + }).fail(function () { + // A project switch cancelled these preferences. Release this + // initialization so the next editor change can initialize Tern. + addFilesDeferred.resolveWith(null); }); } diff --git a/src/editor/MediaViewer.js b/src/editor/MediaViewer.js index 82bdbc8ed1..a252fc1c05 100644 --- a/src/editor/MediaViewer.js +++ b/src/editor/MediaViewer.js @@ -83,7 +83,14 @@ define(function (require, exports, module) { */ function _mediaStreamURL(file) { const platformPath = Phoenix.fs.getTauriPlatformPath(file.fullPath); - return window.PhNodeEngine.mediaURL + + // The file name rides in the path, before the query that actually names the + // file, purely so the URL ends in the right extension. WebKit picks its + // media engine for WebM off the URL extension rather than the Content-Type, + // so without this a .webm plays as "format not supported" on the Mac + // desktop app while the very same bytes play from a blob. mp4 is content + // sniffed and so never showed the problem. Node ignores this segment - it + // matches the route by prefix and reads platformPath from the query. + return window.PhNodeEngine.mediaURL + "/" + encodeURIComponent(file.name) + "?platformPath=" + encodeURIComponent(platformPath); } diff --git a/src/extensions/default/DocCommentHints/integration-tests.js b/src/extensions/default/DocCommentHints/integration-tests.js index f7c836473a..978a020b4b 100644 --- a/src/extensions/default/DocCommentHints/integration-tests.js +++ b/src/extensions/default/DocCommentHints/integration-tests.js @@ -81,7 +81,12 @@ define(function (require, exports, module) { await awaitsForDone(SpecRunnerUtils.openProjectFiles([tc.file]), "open " + tc.file); const editor = EditorManager.getActiveEditor(); editor.setCursorPos(tc.line, tc.ch); - CommandManager.execute(Commands.SHOW_CODE_HINTS); + // Hand the editor to the command. Left to itself it asks for the + // focused editor, and an editor only counts as focused once the + // browser has dispatched focus to it - which it does not do while the + // test window is behind another app. Then no session starts, no popup + // opens, and every case here times out together. + CommandManager.execute(Commands.SHOW_CODE_HINTS, editor); // 1) the code-hints popup appears with our hint await awaitsFor(function () { return $docHint().length > 0; }, diff --git a/src/extensions/default/HTMLCodeHints/integ-tests.js b/src/extensions/default/HTMLCodeHints/integ-tests.js index 3c038c0144..a80521f011 100644 --- a/src/extensions/default/HTMLCodeHints/integ-tests.js +++ b/src/extensions/default/HTMLCodeHints/integ-tests.js @@ -88,15 +88,23 @@ define(function (require, exports, module) { const selected = ProjectManager.getSelectedItem(); expect(selected.fullPath).toBe(testPath + "/jumpToDef.html"); - let editor = EditorManager.getActiveEditor(); - editor.setCursorPos({ line: 5, ch: 6 }); + const hostEditor = EditorManager.getActiveEditor(); + hostEditor.setCursorPos({ line: 5, ch: 6 }); await awaitsForDone(CommandManager.execute(Commands.NAVIGATE_JUMPTO_DEFINITION), "jump to def on div"); - editor = EditorManager.getFocusedInlineEditor(); - expect(editor.document.file.fullPath.endsWith("LiveDevelopment-MultiBrowser-test-files/simpleShared.css")) - .toBeTrue(); + // Ask the host for its inline editors rather than for the focused one: + // an inline editor only holds focus while the window does, so the old + // check came back null whenever the test window was not the OS front + // window, without the inline editor being any less open. + let inlineEditor; + await awaitsFor(function () { + inlineEditor = EditorManager.getInlineEditors(hostEditor)[0]; + return !!inlineEditor; + }, "inline editor to open on the definition"); + expect(inlineEditor.document.file.fullPath + .endsWith("LiveDevelopment-MultiBrowser-test-files/simpleShared.css")).toBeTrue(); await closeSession(); }); @@ -105,14 +113,22 @@ define(function (require, exports, module) { const selected = ProjectManager.getSelectedItem(); expect(selected.fullPath).toBe(testPath + "/jumpToDef.html"); - let editor = EditorManager.getActiveEditor(); - editor.setCursorPos({ line: 6, ch: 23 }); + const hostEditor = EditorManager.getActiveEditor(); + hostEditor.setCursorPos({ line: 6, ch: 23 }); await awaitsForDone(CommandManager.execute(Commands.NAVIGATE_JUMPTO_DEFINITION), "jump to def on div"); - editor = EditorManager.getFocusedInlineEditor(); - expect(editor.document.file.fullPath.endsWith("LiveDevelopment-MultiBrowser-test-files/sub/test.css")) + // Ask the host for its inline editors rather than for the focused one: + // an inline editor only holds focus while the window does, so the old + // check came back null whenever the test window was not the OS front + // window, without the inline editor being any less open. + let inlineEditor; + await awaitsFor(function () { + inlineEditor = EditorManager.getInlineEditors(hostEditor)[0]; + return !!inlineEditor; + }, "inline editor to open on the definition"); + expect(inlineEditor.document.file.fullPath.endsWith("LiveDevelopment-MultiBrowser-test-files/sub/test.css")) .toBeTrue(); await closeSession(); }); @@ -202,9 +218,11 @@ define(function (require, exports, module) { let editor = EditorManager.getActiveEditor(); editor.setCursorPos(cursor); - await awaitsForDone(CommandManager.execute(Commands.SHOW_CODE_HINTS), - "show code hints"); - + // The index has to hold the classes before hints are asked for, not after. + // Showing hints is a one-shot request: made while the index is still + // catching up, the provider has no class hints to offer, no menu opens, + // and nothing asks again once the index is ready - so this passed only + // when the index happened to be warm, and timed out under a full run. await awaitsFor(async function () { for(let hint of expectedSomeHintsArray){ const allSelectors = await CSSUtils.getAllCssSelectorsInProject(); @@ -215,6 +233,11 @@ define(function (require, exports, module) { return true; }, "CSSUtils project selectors to be updated"); + // With the editor passed in, the session does not depend on it holding + // focus, which it does not while the test window is in the background. + await awaitsForDone(CommandManager.execute(Commands.SHOW_CODE_HINTS, editor), + "show code hints"); + await awaitsFor(function () { return $(".codehint-menu").is(":visible"); }, "codehints to be shown"); diff --git a/src/extensions/default/QuickView/unittests.js b/src/extensions/default/QuickView/unittests.js index 440b07d94a..22b31b61b4 100644 --- a/src/extensions/default/QuickView/unittests.js +++ b/src/extensions/default/QuickView/unittests.js @@ -197,7 +197,17 @@ define(function (require, exports, module) { let quickViewSwatch = popoverInfo.content.find("#quick-view-color-swatch"); expect(quickViewSwatch.attr("data-for-test")).toBe(color); quickViewSwatch.click(); - expect(EditorManager.getFocusedInlineWidget()._color).toBe(color); + // Find the color editor among the host's inline widgets rather than as + // the focused one: an inline widget only holds focus while the window + // does, so the old check came back null when the test window was in + // the background, with the color editor open all the same. + let colorWidget; + await awaitsFor(function () { + colorWidget = EditorManager.getActiveEditor().getInlineWidgets() + .find(function (widget) { return widget._color; }); + return !!colorWidget; + }, "inline color editor to open"); + expect(colorWidget._color).toBe(color); }); describe("JavaScript file", function () { diff --git a/src/extensions/default/TypeScriptSupport/unittests.js b/src/extensions/default/TypeScriptSupport/unittests.js index bc8aa1c349..ee8bda0035 100644 --- a/src/extensions/default/TypeScriptSupport/unittests.js +++ b/src/extensions/default/TypeScriptSupport/unittests.js @@ -431,19 +431,21 @@ define(function (require, exports, module) { ExtensionLoader.getRequireContextForExtension("JavaScriptCodeHints")(["main"], resolve, reject); }); let hintText = ""; - await awaitsFor(function () { + await awaitsFor(async function () { if (!jsCodeHints.jsHintProvider.hasHints(editor, null)) { return false; // Tern session/worker may still be starting up } - const response = jsCodeHints.jsHintProvider.getHints(null); - if (!response || typeof response.done !== "function") { - return hintText.indexOf("push") !== -1; // sync response already captured below + // The provider returns a Deferred for fresh hints and an object for + // cached hints. Await either result before checking this request. + let result; + try { + result = await jsCodeHints.jsHintProvider.getHints(null); + } catch (err) { + return false; // A project/session change can cancel a pending request. } - response.done(function (result) { - hintText = ((result && result.hints) || []).map(function (h) { - return $(h).text(); - }).join("|"); - }); + hintText = ((result && result.hints) || []).map(function (h) { + return $(h).text(); + }).join("|"); return hintText.indexOf("push") !== -1; }, "Tern Array-member completions at arr. inside the