ci(e2e): bump TSIO report-upload pin for 504 retries

[yasserfaraazkhan]

Pin test-system-io-report-upload to 19aef73 so screenshot uploads retry on gateway timeouts instead of failing the desktop e2e report group.

NONE

Change Impact: 🟡 Medium

Regression Risk: Login/logout and session_expired handling was changed to ignore signals from non-primary views, and ServerManager.setLoggedIn now short-circuits on unchanged state. This is auth-adjacent and could affect multi-tab/session behavior, but the change is localized to view-driven state updates and is covered by added/updated unit tests.
QA Recommendation: Rely on automated test coverage; run a brief manual smoke test covering login/logout and session-expired behavior triggered from a secondary tab/window to confirm server login state and cookies are only affected from the primary view.
Generated by CodeRabbitAI

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

The PR updates the TSIO report upload action, strengthens E2E tab synchronization, ignores secondary-view logout and expiry events, and prevents redundant server login-state events.

Changes

TSIO action update

Layer / File(s)	Summary
Update TSIO upload action pins .github/workflows/e2e-functional-template.yml, .github/workflows/e2e-functional.yml	The report upload steps now reference test-system-io-report-upload version 0.12.1 using the new pinned commit SHA.

Tab and login-state stability

Layer / File(s)	Summary
Stabilize secondary tab synchronization e2e/specs/menu_bar/window_menu.test.ts	E2E helpers poll for tab creation and readiness, validate registered webContentsIds, and verify secondary tabs remain registered during navigation.
Filter secondary-view login handling src/app/views/webContentsManager.ts, src/app/views/webContentsManager.test.js	Logout and expiry events from non-primary views are ignored, while allowed events continue to update server and app state.
Avoid redundant login-state updates src/common/servers/serverManager.ts, src/common/servers/serverManager.test.js	Repeated login-state assignments return without emitting duplicate state-change events.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ViewManager
  participant webContentsManager
  participant ServerManager
  participant AppState
  ViewManager->>webContentsManager: Receive login or expiry event
  webContentsManager->>ViewManager: Check primary-view status
  webContentsManager->>ServerManager: Update login state when allowed
  webContentsManager->>AppState: Update expiry state when allowed

Loading

Suggested labels: github_actions, dependencies, E2E/Run

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.67% 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 accurately summarizes the TSIO report-upload pin update and its retry motivation, which is a central part of the changeset.
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

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch chore/bump-tsio-report-upload-19aef73

---

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

e2e/specs/menu_bar/window_menu.test.ts (1)

245-252: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Fixed 2s stability loop runs on every createExtraTabs call.

This adds a flat ~2s of wall-clock time to every call to this helper. If createExtraTabs is invoked more than once across the suite, this cost compounds. Consider running this stability check once (e.g., in a dedicated regression test) rather than unconditionally inside the shared helper.
[recommended_refactor]

As per path instructions, e2e guidance states to "Prefer fixes that improve determinism, reduce suite runtime, preserve real-user behavior, and strengthen shared infrastructure over patching individual flaky specs."

🤖 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 `@e2e/specs/menu_bar/window_menu.test.ts` around lines 245 - 252, Move the
repeated 2-second stability loop out of the shared createExtraTabs helper and
run it only once in a dedicated regression test or setup path. Keep
createExtraTabs focused on creating tabs, while preserving the buildServerMap
assertion that verifies secondary Mattermost tabs remain registered.

Source: Path instructions

🤖 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 `@src/app/views/webContentsManager.ts`:
- Around line 180-194: Update handleSessionExpired to mirror the primary-view
guard used in handleTabLoggedIn: when the session-expired event comes from a
non-primary view, return without calling setLoggedIn(view, false). Preserve the
existing expiration handling for the primary view.

---

Nitpick comments:
In `@e2e/specs/menu_bar/window_menu.test.ts`:
- Around line 245-252: Move the repeated 2-second stability loop out of the
shared createExtraTabs helper and run it only once in a dedicated regression
test or setup path. Keep createExtraTabs focused on creating tabs, while
preserving the buildServerMap assertion that verifies secondary Mattermost tabs
remain registered.

🪄 Autofix (Beta)

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: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 22a3ad87-fb25-4244-bea4-9c1bad311ebe

📥 Commits

Reviewing files that changed from the base of the PR and between 387f13b and eb760f6.

📒 Files selected for processing (5)

• e2e/specs/menu_bar/window_menu.test.ts
• src/app/views/webContentsManager.test.js
• src/app/views/webContentsManager.ts
• src/common/servers/serverManager.test.js
• src/common/servers/serverManager.ts

[coderabbitai]

Actionable comments posted: 1

Caution

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

⚠️ Outside diff range comments (1)

e2e/specs/menu_bar/window_menu.test.ts (1)

243-254: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert that the same tabs survive, not only that the count stays high.

createExtraTabs() returns a ServerMap, but the caller discards it, while the helper and post-navigation check validate only tab counts. A tab could be torn down and replaced while the count remains >= 3, allowing this regression test to pass incorrectly. Preserve the initial webContentsId values and assert that they remain present in each rebuilt map; buildServerMap already exposes those IDs.

Suggested assertion change

-async function assertSecondaryTabsRemainRegistered(serverName: string) {
+async function assertSecondaryTabsRemainRegistered(serverName: string, expectedIds: number[]) {
     for (let i = 0; i < 10; i++) {
         await new Promise((resolve) => setTimeout(resolve, 200));
         const map = await buildServerMap(electronApp);
+        const currentIds = (map[serverName] ?? []).map(({webContentsId}) => webContentsId);
+        expect(currentIds).toEqual(expect.arrayContaining(expectedIds));
         expect(
             map[serverName]?.length ?? 0,
             'Secondary Mattermost tabs must remain registered after creation',
         ).toBeGreaterThanOrEqual(3);
     }
 }

-            await createExtraTabs();
-            await assertSecondaryTabsRemainRegistered(windowMenuConfig.servers[0].name);
+            const initialMap = await createExtraTabs();
+            const expectedIds = initialMap[windowMenuConfig.servers[0].name].map(({webContentsId}) => webContentsId);
+            await assertSecondaryTabsRemainRegistered(windowMenuConfig.servers[0].name, expectedIds);

Also applies to: 258-285, 387-390

🤖 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 `@e2e/specs/menu_bar/window_menu.test.ts` around lines 243 - 254, Update the
secondary-tab regression flow around createExtraTabs,
assertSecondaryTabsRemainRegistered, and the post-navigation check to preserve
the initially returned ServerMap and its webContentsId values. Pass those
expected IDs into each validation and assert every rebuilt buildServerMap result
still contains the same IDs, while retaining the existing count checks where
applicable.

🧹 Nitpick comments (1)

e2e/specs/menu_bar/window_menu.test.ts (1)

246-256: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Replace the fixed sleep window with a readiness-based poll.

Ten setTimeout(200) iterations cover exactly two seconds regardless of loading progress. Slow CI runs can still be in the vulnerable loading phase after the loop ends, while fast runs incur an unnecessary fixed delay. Use the existing shell-readiness/polling helpers and continue checking registration until the target views are ready.

🤖 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 `@e2e/specs/menu_bar/window_menu.test.ts` around lines 246 - 256, Update
assertSecondaryTabsRemainRegistered to replace the fixed ten-iteration
setTimeout loop with the existing shell-readiness/polling helpers. Continue
polling the server map until the target secondary views are ready, while
preserving the assertion that at least three tabs remain registered during the
check.

Source: Coding guidelines

🤖 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 `@src/app/views/webContentsManager.test.js`:
- Line 434: Replace the new require() calls for AppState and ipcMain in the
affected tests with ES-module imports in the file’s top-level import block, then
reuse those existing bindings throughout the test cases instead of performing
duplicate Electron or AppState lookups.

---

Outside diff comments:
In `@e2e/specs/menu_bar/window_menu.test.ts`:
- Around line 243-254: Update the secondary-tab regression flow around
createExtraTabs, assertSecondaryTabsRemainRegistered, and the post-navigation
check to preserve the initially returned ServerMap and its webContentsId values.
Pass those expected IDs into each validation and assert every rebuilt
buildServerMap result still contains the same IDs, while retaining the existing
count checks where applicable.

---

Nitpick comments:
In `@e2e/specs/menu_bar/window_menu.test.ts`:
- Around line 246-256: Update assertSecondaryTabsRemainRegistered to replace the
fixed ten-iteration setTimeout loop with the existing shell-readiness/polling
helpers. Continue polling the server map until the target secondary views are
ready, while preserving the assertion that at least three tabs remain registered
during the check.

🪄 Autofix (Beta)

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: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: fb3ef26f-e78c-40f7-b8c4-efca7c15dc99

📥 Commits

Reviewing files that changed from the base of the PR and between eb760f6 and 522038a.

📒 Files selected for processing (3)

• e2e/specs/menu_bar/window_menu.test.ts
• src/app/views/webContentsManager.test.js
• src/app/views/webContentsManager.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/app/views/webContentsManager.ts