Skip to content

Improve UI reliability, performance, and Blacksmith CI - #20

Merged
capy-ai[bot] merged 2 commits into
mainfrom
capy/improve-ui-reliability
Aug 11, 2026
Merged

Improve UI reliability, performance, and Blacksmith CI#20
capy-ai[bot] merged 2 commits into
mainfrom
capy/improve-ui-reliability

Conversation

@zortos293

@zortos293 zortos293 commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add Blacksmith CI for pull requests and main pushes with actionlint, lint, full builds, and Playwright UI regression checks
  • fix fresh-profile stream defaults, QR polling recovery, settings focus/history behavior, tablet navigation, mobile catalog overflow, keyboard card actions, stale catalog/detail requests, and one-shot stream handoffs
  • harden WebRTC setup against stale connections and failed control messages, and stop overlapping ICE polling once connected
  • lazy-load route and settings assets, replace the 13-request My Games dashboard fan-out with the installed-games endpoint, parallelize catalog requests, and remove duplicated helpers and dead APIs/assets/dependencies
  • standardize development on npm, declare direct tooling dependencies, trim packaged files, and isolate release caches by architecture

Verification

  • npm ci --no-audit --no-fund
  • npm run lint
  • npm run build
  • npm run test:ui
  • npx electron-builder --linux dir --x64 --publish never
  • verified the Linux app.asar is 6.5 MB and contains only server runtime dependencies

Summary by CodeRabbit

  • New Features

    • Added responsive improvements for navigation, filters, catalog actions, and mobile layouts.
    • Improved keyboard navigation, focus management, modal behavior, and accessibility labels.
    • Added safeguards for single-use stream launch data.
    • Added lazy loading for faster page transitions.
  • Bug Fixes

    • Prevented stale requests from replacing newer library or installation results.
    • Improved stream connection and reconnection reliability.
    • QR login polling now resumes after temporary errors.
    • Improved settings timer cleanup and navigation behavior.

@cursor

cursor Bot commented Aug 11, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e74d0bbc-3321-42ba-85a3-a34a41dc6577

📥 Commits

Reviewing files that changed from the base of the PR and between 5170be9 and 0d5f802.

📒 Files selected for processing (1)
  • tools/ui-smoke.mjs
🚧 Files skipped from review as they are similar to previous changes (1)
  • tools/ui-smoke.mjs

📝 Walkthrough

Walkthrough

The pull request adds npm-based CI and Playwright smoke tests, narrows public module APIs, updates library and settings interactions, and hardens Electron launch handling plus WebSocket and WebRTC lifecycles.

Changes

Delivery and internal boundaries

Layer / File(s) Summary
npm delivery and CI setup
.github/workflows/*, package.json, README.md, electron/preload.*, index.html
CI, documentation, package metadata, Electron packaging, preload logging, and release cache keys now use the updated npm-based setup.
Internal API and session boundaries
server/*, src/api/*, src/auth/*, src/lib/*, src/types/index.ts
Unused exports and legacy endpoint helpers were removed. Upstream cookie-token creation and persistence flows were removed. Settings parsing now handles empty storage values explicitly.

UI and streaming

Layer / File(s) Summary
UI routing, library, and modal flows
src/App.tsx, src/pages/*, src/components/*, src/layouts/*, src/styles.css, tools/ui-smoke.mjs
Routes now load lazily. Library and install requests ignore stale responses. Settings focus, responsive controls, timers, accessibility, and Playwright smoke coverage were updated.
Single-use launch and connection lifecycle
electron/main.ts, src/pages/StreamPage.tsx, src/stream/*
Launch payloads are consumed once. Stream connections use generation guards, terminal failures, safer keyboard handling, and generation-aware ICE polling.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

Suggested labels: capy

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the pull request's main UI reliability, performance, and Blacksmith CI changes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch capy/improve-ui-reliability

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/stream/OpenStroidStreamClient.ts (1)

586-617: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Reserve the connection generation before the first await.

Line 588 captures connectionGeneration after await this.disconnect(true). If reconnect starts while another connection is pending, both connect calls can resume and capture the same generation. Both calls then pass the stale checks on Lines 610 and 615.

A stale call can open a control socket and leave it open after a newer call replaces this.ws. Allocate the generation synchronously at connect entry. Let its internal cleanup avoid another increment. Bind the generation to openControlWebSocket and close the socket when it becomes stale.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/stream/OpenStroidStreamClient.ts` around lines 586 - 617, Update connect
to reserve a unique connection generation synchronously before its first await,
and ensure the initial disconnect cleanup does not increment that reserved
generation. Pass this generation through openControlWebSocket, reject stale work
before and after socket creation, and close any socket opened by a stale connect
before returning. Use the existing connectionGeneration checks and websocket
lifecycle symbols without changing current behavior for the active connection.
🧹 Nitpick comments (3)
src/components/SettingsModalHost.tsx (1)

29-38: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Guard the focus restore against a detached element.

The recorded element can leave the DOM while the modal is open, for example when the background route re-renders its navigation. focus() on a detached element silently does nothing and focus falls back to document.body. Check isConnected and fall back to a known element.

♻️ Proposed change
     return () => {
       appRoot?.removeAttribute('inert');
-      previousFocus.current?.focus();
+      const target = previousFocus.current;
+      if (target?.isConnected) {
+        target.focus();
+      }
+      previousFocus.current = null;
     };
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/SettingsModalHost.tsx` around lines 29 - 38, Update the
cleanup logic in the useEffect within SettingsModalHost so it restores focus
only when previousFocus.current is connected to the document; otherwise focus
the established fallback element. Preserve the existing inert attribute cleanup
and modal-open focus behavior.
src/App.tsx (1)

14-19: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Add an error boundary around the lazy routes.

lazy() rejects when a dynamic import fails, for example after a deploy replaces hashed chunks or when the network drops. Suspense does not catch that rejection, so the whole app tree unmounts and the user sees a blank screen. Wrap the Suspense boundary in an error boundary that shows a retry action.

♻️ Proposed structure
-          <Suspense fallback={<Center h="100vh"><Loader color="brand" type="dots" /></Center>}>
-            <AppRoutes />
-          </Suspense>
+          <RouteErrorBoundary>
+            <Suspense fallback={<Center h="100vh"><Loader color="brand" type="dots" /></Center>}>
+              <AppRoutes />
+            </Suspense>
+          </RouteErrorBoundary>

RouteErrorBoundary can be a small class component that renders a reload button on componentDidCatch.

Also applies to: 27-29

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/App.tsx` around lines 14 - 19, Wrap the lazy route Suspense tree in a
RouteErrorBoundary class component that catches dynamic-import failures and
renders a retry/reload action instead of leaving the app blank. Define the
boundary near the lazy page declarations or route setup, and ensure all lazy
routes including LoginPage, MyGamesPage, LibraryCatalogPage, InstallPage,
SettingsPage, and StreamPage are covered.
src/pages/InstallPage.tsx (1)

241-258: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Duplicated game-detail request logic across two pages. Both pages implement the same generation counter, the same getGameDetails(...).catch(() => null) merge, and the same invalidating close handler. The root cause is copied logic rather than a shared abstraction; a future fix to the guard must be applied twice.

  • src/pages/InstallPage.tsx#L241-L258: replace openDetails, closeDetails, and detailsRequestId with a shared useGameDetails() hook exported from src/lib/.
  • src/pages/LibraryPage.tsx#L129-L146: consume the same hook and delete the local copies, keeping the existing isDetailLoading naming through the hook return value.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pages/InstallPage.tsx` around lines 241 - 258, The game-detail request
and invalidation logic is duplicated across both pages. In
src/pages/InstallPage.tsx lines 241-258, create and consume a shared
useGameDetails hook exported from src/lib/, replacing detailsRequestId,
openDetails, and closeDetails; in src/pages/LibraryPage.tsx lines 129-146,
consume the same hook and remove the local implementations while preserving the
existing isDetailLoading name through the hook’s return value.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/ci.yml:
- Around line 34-39: Update the “Setup Node.js” CI step to install the declared
npm@11.16.0 after Node.js setup, then assert the active version with npm
--version before the npm ci step. Ensure the workflow uses npm 11.16.0
consistently for dependency installation.

In `@src/pages/LibraryPage.tsx`:
- Around line 77-90: Update fetchLibrary in LibraryPage to use a
libraryRequestId generation guard: increment and capture the request ID before
awaiting getInstalledGames, and only apply game-list, load-state, and error
updates when the response matches the latest generation. Add an unmount cleanup
effect that increments libraryRequestId.current to invalidate pending requests
after unmount, following the existing InstallPage pattern.

In `@src/pages/LoginPage.tsx`:
- Around line 139-144: Update the QR polling catch/reschedule logic in LoginPage
to verify that qrSessionRef.current still represents the rejected request’s
sessionId before creating a timer, so an in-flight cancelled session cannot
overwrite the new session’s timer. Also bound consecutive polling failures by
applying backoff and stopping after the configured failure limit or when
activeSession.timeoutAt has passed, while preserving normal polling for valid,
non-terminal sessions.

In `@tools/ui-smoke.mjs`:
- Around line 111-114: Restore the fresh-profile default stream bitrate to 20
Mbps in the settings initialization/defaults flow used by the Settings dialog.
Ensure an unset profile receives this value while preserving explicit
user-configured bitrates, so the existing 20 Mbps assertion remains valid.

---

Outside diff comments:
In `@src/stream/OpenStroidStreamClient.ts`:
- Around line 586-617: Update connect to reserve a unique connection generation
synchronously before its first await, and ensure the initial disconnect cleanup
does not increment that reserved generation. Pass this generation through
openControlWebSocket, reject stale work before and after socket creation, and
close any socket opened by a stale connect before returning. Use the existing
connectionGeneration checks and websocket lifecycle symbols without changing
current behavior for the active connection.

---

Nitpick comments:
In `@src/App.tsx`:
- Around line 14-19: Wrap the lazy route Suspense tree in a RouteErrorBoundary
class component that catches dynamic-import failures and renders a retry/reload
action instead of leaving the app blank. Define the boundary near the lazy page
declarations or route setup, and ensure all lazy routes including LoginPage,
MyGamesPage, LibraryCatalogPage, InstallPage, SettingsPage, and StreamPage are
covered.

In `@src/components/SettingsModalHost.tsx`:
- Around line 29-38: Update the cleanup logic in the useEffect within
SettingsModalHost so it restores focus only when previousFocus.current is
connected to the document; otherwise focus the established fallback element.
Preserve the existing inert attribute cleanup and modal-open focus behavior.

In `@src/pages/InstallPage.tsx`:
- Around line 241-258: The game-detail request and invalidation logic is
duplicated across both pages. In src/pages/InstallPage.tsx lines 241-258, create
and consume a shared useGameDetails hook exported from src/lib/, replacing
detailsRequestId, openDetails, and closeDetails; in src/pages/LibraryPage.tsx
lines 129-146, consume the same hook and remove the local implementations while
preserving the existing isDetailLoading name through the hook’s return value.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 19305f3b-7af2-4dcd-b784-16cb60bebef9

📥 Commits

Reviewing files that changed from the base of the PR and between 51426a2 and 5170be9.

⛔ Files ignored due to path filters (3)
  • bun.lock is excluded by !**/*.lock
  • package-lock.json is excluded by !**/package-lock.json
  • public/icons.svg is excluded by !**/*.svg
📒 Files selected for processing (35)
  • .github/workflows/ci.yml
  • .github/workflows/release.yml
  • README.md
  • electron/main.ts
  • electron/preload.cjs
  • electron/preload.cts
  • electron/preload.js
  • index.html
  • package.json
  • server/app.ts
  • server/lib/qrLogin.ts
  • server/lib/session.ts
  • server/lib/upstream.ts
  • src/App.tsx
  • src/api/config.ts
  • src/api/endpoints.ts
  • src/api/index.ts
  • src/auth/index.ts
  • src/auth/storage.ts
  • src/components/MotionProvider.tsx
  • src/components/SettingsModalHost.tsx
  • src/layouts/AuthenticatedLayout.tsx
  • src/lib/gameUtils.ts
  • src/lib/userSettings.ts
  • src/pages/InstallPage.tsx
  • src/pages/LibraryPage.tsx
  • src/pages/LoginPage.module.css
  • src/pages/LoginPage.tsx
  • src/pages/SettingsPage.tsx
  • src/pages/StreamPage.tsx
  • src/stream/GamepadController.ts
  • src/stream/OpenStroidStreamClient.ts
  • src/styles.css
  • src/types/index.ts
  • tools/ui-smoke.mjs
💤 Files with no reviewable changes (9)
  • index.html
  • electron/preload.js
  • src/pages/LoginPage.module.css
  • src/components/MotionProvider.tsx
  • src/auth/index.ts
  • electron/preload.cts
  • src/api/config.ts
  • electron/preload.cjs
  • src/api/index.ts

Comment thread .github/workflows/ci.yml
Comment on lines +34 to +39
- name: Setup Node.js
uses: actions/setup-node@v4
with:
node-version: 22
cache: npm
cache-dependency-path: package-lock.json

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 2 '"packageManager": "npm@|node-version:|corepack|npm install --global|npm --version' \
  package.json .github/workflows/ci.yml

Repository: OpenCloudGaming/OpenStroid

Length of output: 673


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- workflow ---'
cat -n .github/workflows/ci.yml

printf '%s\n' '--- package metadata and lockfile version ---'
cat -n package.json | sed -n '1,80p'
head -n 12 package-lock.json

printf '%s\n' '--- npm-related repository configuration ---'
rg -n -S 'packageManager|engines|engine-strict|npm@|corepack|npm ci|npm install|npm --version' \
  package.json package-lock.json .github README.md 2>/dev/null || true

printf '%s\n' '--- installed tool availability ---'
node --version 2>/dev/null || true
npm --version 2>/dev/null || true
corepack --version 2>/dev/null || true

Repository: OpenCloudGaming/OpenStroid

Length of output: 23806


🌐 Web query:

Node.js 22 bundled npm version official release documentation npm 11 supported Node.js versions

💡 Result:

Node.js 22 does not bundle npm 11 by default. Node.js 22 continues to ship with npm 10 [1]. npm 11 was introduced as the bundled package manager starting with Node.js 24 [2]. Regarding compatibility, npm 11 explicitly supports Node.js versions ^20.17.0 or >=22.9.0 [3][4][5]. Users running older versions of Node.js 22 (e.g., 22.5.1) who attempt to manually update to npm 11 will receive a warning that their Node.js environment is not supported by that version of npm [5]. Additionally, users should be aware of a known regression affecting npm installations on Node.js 22.22.2, which caused npm commands to fail with a MODULE_NOT_FOUND error due to a missing promise-retry dependency [6][7]. This issue is specific to the 22.22.2 release [6][7]. Official recommendations for resolving this or managing npm versions in Node.js 22 include pinning the Node.js version to 22.22.1 or using corepack to manage the npm version [6].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
import json
from pathlib import Path

package = json.loads(Path("package.json").read_text())
lock = json.loads(Path("package-lock.json").read_text())

print("packageManager:", package.get("packageManager"))
print("package engines:", package.get("engines"))
print("lockfileVersion:", lock.get("lockfileVersion"))
print("workflow npm commands:")
for i, line in enumerate(Path(".github/workflows/ci.yml").read_text().splitlines(), 1):
    if "npm" in line or "node-version" in line or "setup-node" in line:
        print(f"{i}: {line}")
PY

Repository: OpenCloudGaming/OpenStroid

Length of output: 515


🌐 Web query:

site:nodejs.org Node.js v22 downloads npm version bundled

💡 Result:

The npm version bundled with Node.js v22 varies depending on the specific sub-version of the Node.js v22 release line. Because Node.js releases are updated periodically to include newer versions of npm, you can find the bundled npm version by checking the release documentation for your specific v22.x version [1][2][3][4]. For example, earlier releases in the v22.x line bundled older npm versions, such as v10.7.0 in Node.js v22.2.0 [3], while later releases, such as v22.22.2 and v22.22.3, bundle npm v10.9.7 and v10.9.8, respectively [2][4]. To verify the npm version currently installed with your specific version of Node.js, you can run the following command in your terminal: npm -v For the most accurate and up-to-date information regarding your specific installation, please refer to the official Node.js download archives for the v22.x line [1].

Citations:


Use npm 11.16.0 in CI.

Node.js 22 bundles npm 10.x, while package.json declares npm@11.16.0. Install npm 11.16.0 and assert npm --version before npm ci to keep the CI toolchain reproducible.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/ci.yml around lines 34 - 39, Update the “Setup Node.js” CI
step to install the declared npm@11.16.0 after Node.js setup, then assert the
active version with npm --version before the npm ci step. Ensure the workflow
uses npm 11.16.0 consistently for dependency installation.

Comment thread src/pages/LibraryPage.tsx
Comment on lines +77 to 90
const fetchLibrary = useCallback(async () => {
setLoadState('loading');
setErrorMsg('');
try {
const data = await getLibraryDashboard();
setDashboard({
...EMPTY_DASHBOARD,
...data,
facets: { ...EMPTY_DASHBOARD.facets, ...data.facets },
account: { ...EMPTY_DASHBOARD.account, ...data.account },
sessions: { ...EMPTY_DASHBOARD.sessions, ...data.sessions },
});
setGames(await getInstalledGames());
setLoadState('success');
} catch (err: unknown) {
const msg =
(err as { response?: { data?: { message?: string } } })?.response?.data?.message ||
'Failed to load the Boosteroid dashboard data.';
'Failed to load your Boosteroid library.';
setErrorMsg(msg);
setLoadState('error');
}
}, []);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Add a request-generation guard to fetchLibrary.

openDetails in this file and loadGames in src/pages/InstallPage.tsx both guard against stale responses, but fetchLibrary does not. The refresh button, the initial-load effect, and handleSync can all start fetchLibrary concurrently. A slower earlier response then overwrites the newer game list and sets loadState to success. The same call also updates state after unmount.

🐛 Proposed fix
+  const libraryRequestId = useRef(0);
+
   const fetchLibrary = useCallback(async () => {
+    const requestId = ++libraryRequestId.current;
     setLoadState('loading');
     setErrorMsg('');
     try {
-      setGames(await getInstalledGames());
+      const next = await getInstalledGames();
+      if (libraryRequestId.current !== requestId) return;
+      setGames(next);
       setLoadState('success');
     } catch (err: unknown) {
+      if (libraryRequestId.current !== requestId) return;
       const msg =
         (err as { response?: { data?: { message?: string } } })?.response?.data?.message ||
         'Failed to load your Boosteroid library.';
       setErrorMsg(msg);
       setLoadState('error');
     }
   }, []);

Also increment libraryRequestId.current in an unmount cleanup effect, as src/pages/InstallPage.tsx does at lines 110-113.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pages/LibraryPage.tsx` around lines 77 - 90, Update fetchLibrary in
LibraryPage to use a libraryRequestId generation guard: increment and capture
the request ID before awaiting getInstalledGames, and only apply game-list,
load-state, and error updates when the response matches the latest generation.
Add an unmount cleanup effect that increments libraryRequestId.current to
invalidate pending requests after unmount, following the existing InstallPage
pattern.

Comment thread src/pages/LoginPage.tsx
Comment on lines +139 to +144
const activeSession = qrSessionRef.current;
if (activeSession && !QR_TERMINAL_STATUSES.has(activeSession.status)) {
qrPollHandle.current = window.setTimeout(() => {
void pollQRCodeStatusRef.current(sessionId);
}, activeSession.pollIntervalMs || DEFAULT_QR_POLL_INTERVAL_MS);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Check the session id before you reschedule, and bound the retry loop.

Two problems in this catch block.

  1. Stale session. The rescheduled poll captures the old sessionId. If the user presses "New Code" while a status request is in flight, startQRCodeFlow calls stopQRCodePolling() (nothing pending to clear at that moment) and stores a new timer in qrPollHandle.current at line 164. The old request then rejects, and this block overwrites qrPollHandle.current with a timer for the cancelled session. The new session is never polled and login stalls. qrSessionRef.current already holds the new session, so an id comparison closes the gap.

  2. Unbounded retries. qrSessionRef.current.status only advances on a successful response. If the endpoint stays down, the status remains polling and the client retries at a fixed interval until the page unmounts. Add a bounded backoff and stop after a consecutive-failure limit, or stop once activeSession.timeoutAt has passed.

🐛 Proposed fix for the stale-session reschedule
       const activeSession = qrSessionRef.current;
-      if (activeSession && !QR_TERMINAL_STATUSES.has(activeSession.status)) {
+      if (
+        activeSession
+        && activeSession.id === sessionId
+        && !QR_TERMINAL_STATUSES.has(activeSession.status)
+      ) {
         qrPollHandle.current = window.setTimeout(() => {
           void pollQRCodeStatusRef.current(sessionId);
         }, activeSession.pollIntervalMs || DEFAULT_QR_POLL_INTERVAL_MS);
       }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const activeSession = qrSessionRef.current;
if (activeSession && !QR_TERMINAL_STATUSES.has(activeSession.status)) {
qrPollHandle.current = window.setTimeout(() => {
void pollQRCodeStatusRef.current(sessionId);
}, activeSession.pollIntervalMs || DEFAULT_QR_POLL_INTERVAL_MS);
}
const activeSession = qrSessionRef.current;
if (
activeSession
&& activeSession.id === sessionId
&& !QR_TERMINAL_STATUSES.has(activeSession.status)
) {
qrPollHandle.current = window.setTimeout(() => {
void pollQRCodeStatusRef.current(sessionId);
}, activeSession.pollIntervalMs || DEFAULT_QR_POLL_INTERVAL_MS);
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pages/LoginPage.tsx` around lines 139 - 144, Update the QR polling
catch/reschedule logic in LoginPage to verify that qrSessionRef.current still
represents the rejected request’s sessionId before creating a timer, so an
in-flight cancelled session cannot overwrite the new session’s timer. Also bound
consecutive polling failures by applying backoff and stopping after the
configured failure limit or when activeSession.timeoutAt has passed, while
preserving normal polling for valid, non-terminal sessions.

Comment thread tools/ui-smoke.mjs Outdated
Comment on lines +111 to +114
await page.goto(`${origin}/settings`);
await page.getByRole('dialog', { name: 'Settings' }).waitFor();
assert.equal(await page.getByText('20 Mbps', { exact: true }).count(), 1, 'Fresh profiles did not use the default stream bitrate');
assert.equal(await page.getByText('70%', { exact: true }).count(), 1, 'Fresh profiles did not use the default stream volume');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Restore the fresh-profile stream bitrate default.

Line 113 currently fails in CI because the Settings dialog does not show 20 Mbps for a fresh profile. Restore the unset-profile bitrate default to 20 Mbps. Do not weaken this assertion because the PR objective requires that default.

🧰 Tools
🪛 GitHub Actions: CI / 0_lint-build-ui.txt

[error] 113-113: UI smoke test failed: Fresh profiles did not use the default stream bitrate. Assertion expected 1 but received 0. Command 'npm run test:ui' failed with exit code 1.

🪛 GitHub Actions: CI / lint-build-ui

[error] 113-113: UI smoke test failed: fresh profiles did not use the default stream bitrate. Assertion expected 1 but received 0. Command 'npm run test:ui' failed with exit code 1.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tools/ui-smoke.mjs` around lines 111 - 114, Restore the fresh-profile default
stream bitrate to 20 Mbps in the settings initialization/defaults flow used by
the Settings dialog. Ensure an unset profile receives this value while
preserving explicit user-configured bitrates, so the existing 20 Mbps assertion
remains valid.

Source: Pipeline failures

@capy-ai
capy-ai Bot merged commit 990e9b0 into main Aug 11, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant