From f28b1e7a6b7beed5f2b267ebbade7a8bc2c4b3d6 Mon Sep 17 00:00:00 2001 From: san Date: Sat, 7 Nov 2026 12:12:29 +0100 Subject: [PATCH] [ryoku] launcher: fix race condition causing empty app list at startup DesktopEntries.applications loads asynchronously when Quickshell starts. If the launcher was opened before the scan of /usr/share/applications completed, loadApps() received an empty array but appsLoaded was still set to true in two callers (Component.onCompleted and onIsVisibleChanged), preventing any subsequent retry. Root causes fixed: 1. Component.onCompleted: unconditional appsLoaded = true after loadApps() overrode the empty-result state set inside loadApps(). Removed. 2. onIsVisibleChanged show-branch: same unconditional override after loadApps() call. Removed. 3. loadApps() flag ownership: the function now sets appsLoaded=true when finalArr.length > 0, and appsLoaded=false when empty, in both branches. Callers no longer touch the flag. Result: on every launcher open, if allApps is still empty the next close-and-reopen or any DesktopEntries signal emitted while the launcher is closed sets appsLoaded=false, so the next open retries. No timers, no polling, zero overhead. Adds appindex.test.mjs: 10 regression tests covering the race condition, evaluating Launcher.qml handlers directly via node:vm. Red-green verified: tests fail when the original bugs are restored. Note: Fixed: the app launcher recovers when opened before desktop entries finish scanning --- ryoku/CHANGELOG.md | 5 + .../barstyles/python/launcher/Launcher.qml | 16 +- .../python/launcher/appindex.test.mjs | 248 ++++++++++++++++++ 3 files changed, 266 insertions(+), 3 deletions(-) create mode 100644 ryoku/shell/quickshell/shell/modules/bar/barstyles/python/launcher/appindex.test.mjs diff --git a/ryoku/CHANGELOG.md b/ryoku/CHANGELOG.md index ac29baba3..9d5b629ea 100644 --- a/ryoku/CHANGELOG.md +++ b/ryoku/CHANGELOG.md @@ -3,6 +3,11 @@ ## Unreleased ### Fixed +- **The app launcher recovers when opened before desktop entries finish scanning.** + `loadApps()` is now the sole authority over the `appsLoaded` flag; callers + no longer override it unconditionally. If `DesktopEntries` has not finished + scanning at the time the launcher opens, `appsLoaded` stays `false` and the + next open or entry update retries (`python/launcher/Launcher.qml`). - Fix with AI can use Codex with a repair-specific model and reasoning effort. Terminal launches now precede the health scan so progress and errors stay visible. - **Adaptive sync keeps its saved state in Displays.** Hyprland and niri now diff --git a/ryoku/shell/quickshell/shell/modules/bar/barstyles/python/launcher/Launcher.qml b/ryoku/shell/quickshell/shell/modules/bar/barstyles/python/launcher/Launcher.qml index 09846dcba..1e6f2b5a8 100644 --- a/ryoku/shell/quickshell/shell/modules/bar/barstyles/python/launcher/Launcher.qml +++ b/ryoku/shell/quickshell/shell/modules/bar/barstyles/python/launcher/Launcher.qml @@ -144,7 +144,7 @@ PanelWindow { rankFetcher.running = true; } loadApps(); - appsLoaded = true; + // Note: appsLoaded is managed inside loadApps() - do NOT set it here if (currentTabIndex === 0) { executeFilter(""); } else { @@ -518,9 +518,10 @@ PanelWindow { if (isVisible) { tabSwitch.currentIndex = launcherWindow.currentTabIndex; restartItemsIntro(); - if (!launcherWindow.appsLoaded) { + // Attempt to load if not loaded yet, or if previous load came back empty (race condition at startup) + if (!launcherWindow.appsLoaded || launcherWindow.allApps.length === 0) { launcherWindow.loadApps(); - launcherWindow.appsLoaded = true; + // Note: appsLoaded is set inside loadApps() to avoid overriding an empty-result early return } if (searchInput.text !== "") { searchInput.clear(); @@ -732,6 +733,15 @@ PanelWindow { } launcherWindow.allApps = finalArr; + + // loadApps() is the sole authority over appsLoaded. + // Only mark loaded when we actually have entries; otherwise keep false so the + // next visibility event retries (handles async DesktopEntries race condition). + if (finalArr.length > 0) { + launcherWindow.appsLoaded = true; + } else { + launcherWindow.appsLoaded = false; + } } ListModel { diff --git a/ryoku/shell/quickshell/shell/modules/bar/barstyles/python/launcher/appindex.test.mjs b/ryoku/shell/quickshell/shell/modules/bar/barstyles/python/launcher/appindex.test.mjs new file mode 100644 index 000000000..dd82232df --- /dev/null +++ b/ryoku/shell/quickshell/shell/modules/bar/barstyles/python/launcher/appindex.test.mjs @@ -0,0 +1,248 @@ +/** + * Regression tests for the launcher app-index race condition. + * + * These tests directly exercise the JavaScript handlers extracted from Launcher.qml + * using node:vm, matching the pattern used by other QML unit tests in the repo: + * - loadApps() + * - Component.onCompleted + * - onIsVisibleChanged + * - DesktopEntries Connections (onValuesChanged) + * + * Bug: launcher appeared empty when opened before DesktopEntries finished + * scanning /usr/share/applications. The fix makes loadApps() the sole + * authority over `appsLoaded`, and onIsVisibleChanged retries when empty. + * + * Run with: node --test appindex.test.mjs + */ + +import assert from "node:assert/strict"; +import test from "node:test"; + +// --------------------------------------------------------------------------- +// Direct evaluation of Launcher.qml handlers via vm +// --------------------------------------------------------------------------- + +import fs from "node:fs"; +import vm from "node:vm"; + +const qmlSource = fs.readFileSync(new URL("./Launcher.qml", import.meta.url), "utf8"); + +function extractBlock(source, marker) { + const startIdx = source.indexOf(marker); + if (startIdx === -1) throw new Error("Could not find " + marker + " in Launcher.qml"); + const braceStart = source.indexOf("{", startIdx); + let depth = 1; + let i = braceStart + 1; + while (i < source.length && depth > 0) { + if (source[i] === "{") depth++; + else if (source[i] === "}") depth--; + i++; + } + return source.slice(braceStart + 1, i - 1); +} + +const loadAppsBody = extractBlock(qmlSource, "function loadApps()"); +const onCompletedBody = extractBlock(qmlSource, "Component.onCompleted:"); +const onIsVisibleChangedBody = extractBlock(qmlSource, "onIsVisibleChanged:"); +const onValuesChangedBody = extractBlock(qmlSource, "function onValuesChanged()"); + +function makeState(desktopEntries = []) { + const ctx = { + launcherWindow: null, + appsLoaded: false, + allApps: [], + isVisible: false, + currentTabIndex: 0, + smartRanking: false, + usageRanks: {}, + DesktopEntries: { + applications: { + values: desktopEntries + } + }, + WindowRegistry: undefined, + rankFetcher: { running: false }, + searchInput: { text: "", clear() { this.text = ""; } }, + tabSwitch: { currentIndex: 0 }, + restartItemsIntro() {}, + executeFilter(q) {}, + executeFileSearch(q) {}, + grabInputFocus() {}, + focusTimer: { restart() {}, stop() {} }, + focusRetryTimer: { restart() {}, stop() {} }, + focusFinalTimer: { restart() {}, stop() {} }, + keyboardNavTimer: { stop() {} }, + itemsIntroSequence: { stop() {} }, + appList: { resetScroll() {} }, + filterDebounceTimer: { stop() {} }, + fileDebounceTimer: { stop() {} }, + fileSearchProcess: { running: false }, + introItems: 0.0, + }; + ctx.launcherWindow = ctx; + vm.createContext(ctx); + + ctx.loadApps = vm.runInContext(`(function() { ${loadAppsBody} })`, ctx); + const fnOnCompleted = vm.runInContext(`(function() { ${onCompletedBody} })`, ctx); + const fnOnIsVisibleChanged = vm.runInContext(`(function(isVisible) { + launcherWindow.isVisible = isVisible; + ${onIsVisibleChangedBody} + })`, ctx); + const fnOnValuesChanged = vm.runInContext(`(function() { ${onValuesChangedBody} })`, ctx); + + return { + ctx, + loadApps: () => ctx.loadApps.call(ctx), + onCompleted: () => fnOnCompleted.call(ctx), + setVisible: (v) => fnOnIsVisibleChanged.call(ctx, v), + onDesktopEntriesChanged: (entries) => { + ctx.DesktopEntries.applications.values = entries; + fnOnValuesChanged.call(ctx); + } + }; +} + +// --------------------------------------------------------------------------- +// Happy-path tests +// --------------------------------------------------------------------------- + +test("normal boot: entries ready before onCompleted -> appsLoaded=true", () => { + const apps = [{ name: "Firefox", id: "firefox.desktop", noDisplay: false }]; + const inst = makeState(apps); + inst.onCompleted(); + + assert.equal(inst.ctx.appsLoaded, true); + assert.equal(inst.ctx.allApps.length, 1); +}); + +test("normal open: appsLoaded=true, opens fine", () => { + const apps = [{ name: "Firefox", id: "firefox.desktop", noDisplay: false }]; + const inst = makeState(apps); + inst.onCompleted(); + inst.setVisible(true); + + assert.equal(inst.ctx.appsLoaded, true); + assert.equal(inst.ctx.allApps.length, 1); +}); + +// --------------------------------------------------------------------------- +// Race condition regression tests +// --------------------------------------------------------------------------- + +test("[REGRESSION] race: DesktopEntries empty at onCompleted must NOT set appsLoaded=true", () => { + // DesktopEntries not yet scanned at component creation + const inst = makeState([]); // empty + inst.onCompleted(); + + // appsLoaded must stay false - the original bug set it to true here + assert.equal(inst.ctx.appsLoaded, false, "appsLoaded must be false when entries are empty"); + assert.equal(inst.ctx.allApps.length, 0); +}); + +test("[REGRESSION] race: opening launcher while DesktopEntries still empty retries on next open", () => { + const inst = makeState([]); // DesktopEntries not ready + inst.onCompleted(); // appsLoaded=false (empty) + + // User opens launcher before entries are available + inst.setVisible(true); + assert.equal(inst.ctx.allApps.length, 0, "still empty: entries not loaded yet"); + assert.equal(inst.ctx.appsLoaded, false, "appsLoaded must remain false when opening empty"); + + // DesktopEntries finishes scanning while launcher is closed + inst.setVisible(false); // hide resets appsLoaded=false + inst.onDesktopEntriesChanged([ // entries now available + { name: "Firefox", id: "firefox.desktop", noDisplay: false }, + { name: "Terminal", id: "terminal.desktop", noDisplay: false }, + ]); + + // User opens launcher again + inst.setVisible(true); + assert.equal(inst.ctx.appsLoaded, true, "must be loaded on second open"); + assert.equal(inst.ctx.allApps.length, 2, "apps must be populated on second open"); +}); + +test("[REGRESSION] race: opening while empty retries immediately on same open if entries arrive mid-session", () => { + const inst = makeState([]); + inst.onCompleted(); + inst.setVisible(true); // opens empty + + assert.equal(inst.ctx.allApps.length, 0); + + // DesktopEntries emits signal while launcher is OPEN + inst.onDesktopEntriesChanged([ + { name: "Firefox", id: "firefox.desktop", noDisplay: false }, + ]); + + assert.equal(inst.ctx.appsLoaded, true, "reactive refresh while open must set appsLoaded=true"); + assert.equal(inst.ctx.allApps.length, 1, "reactive refresh while open must populate allApps"); +}); + +test("[REGRESSION] noDisplay entries are excluded and must not mark as loaded", () => { + // All entries are noDisplay=true: effectively empty from the launcher's POV + const inst = makeState([ + { name: "Hidden App", id: "hidden1.desktop", noDisplay: true }, + { name: "Another Hidden", id: "hidden2.desktop", noDisplay: true }, + ]); + inst.onCompleted(); + + assert.equal(inst.ctx.appsLoaded, false, "noDisplay-only result must not mark as loaded"); + assert.equal(inst.ctx.allApps.length, 0); +}); + +test("[REGRESSION] onIsVisibleChanged retries if allApps empty even when appsLoaded=true", () => { + // Simulate the state left by the original bug: appsLoaded=true but allApps=[] + // This can happen if external code (not loadApps) sets appsLoaded=true. + const inst = makeState([{ name: "Firefox", id: "firefox.desktop", noDisplay: false }]); + inst.ctx.appsLoaded = true; + inst.ctx.allApps = []; // corrupted state from old bug + + inst.setVisible(true); + + assert.equal(inst.ctx.appsLoaded, true, "after retry with real entries, must be loaded"); + assert.equal(inst.ctx.allApps.length, 1, "must have recovered apps from corrupted state"); +}); + +// --------------------------------------------------------------------------- +// Edge-case tests +// --------------------------------------------------------------------------- + +test("hide always resets appsLoaded so next show gets fresh data", () => { + const apps = [{ name: "Firefox", id: "firefox.desktop", noDisplay: false }]; + const inst = makeState(apps); + inst.onCompleted(); + inst.setVisible(true); + assert.equal(inst.ctx.appsLoaded, true); + + inst.setVisible(false); + assert.equal(inst.ctx.appsLoaded, false, "hide must reset appsLoaded"); +}); + +test("rapid open-close-open cycle always shows apps if entries are available", () => { + const apps = [ + { name: "Firefox", id: "firefox.desktop", noDisplay: false }, + { name: "Chromium", id: "chromium.desktop", noDisplay: false } + ]; + const inst = makeState(apps); + inst.onCompleted(); + + for (let i = 0; i < 10; i++) { + inst.setVisible(true); + assert.equal(inst.ctx.allApps.length, 2, `open #${i + 1} must show apps`); + inst.setVisible(false); + } +}); + +test("DesktopEntries changed while visible reloads without requiring hide/show", () => { + const inst = makeState([{ name: "Firefox", id: "firefox.desktop", noDisplay: false }]); + inst.onCompleted(); + inst.setVisible(true); + assert.equal(inst.ctx.allApps.length, 1); + + // A new app is installed while launcher is open + inst.onDesktopEntriesChanged([ + { name: "Firefox", id: "firefox.desktop", noDisplay: false }, + { name: "NewApp", id: "newapp.desktop", noDisplay: false }, + ]); + + assert.equal(inst.ctx.allApps.length, 2, "must include newly installed app without hide/show"); +});