packages/app/e2e/sidebar/sidebar-session-links.spec.ts (1)

23-23: ⚡ Quick win

Prefer shared route assertion helpers over raw slug regexes

These route checks interpolate slug directly in regexes. For cross-platform reliability (notably Windows canonicalization), route assertions should go through the shared helpers in ../actions rather than raw URL regex construction.

As per coding guidelines: "When validating routing, assert against canonical or resolved workspace slugs using shared helpers from ../actions to account for Windows canonicalization."

Also applies to: 30-30

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/app/e2e/sidebar/sidebar-session-links.spec.ts` at line 23, The test
uses a raw regex with interpolated slug in the page.toHaveURL assertion
(expect(page).toHaveURL(new RegExp(`/${slug}/session/${two.id}...`))), which can
fail on Windows; replace this by importing and using the shared route
assertion/build helper(s) from ../actions (the canonical/resolved workspace slug
helper) to construct or assert the session URL for slug and two.id, and update
both occurrences (the current line and the similar one at 30) to call that
helper instead of building the regex inline so routing checks use the shared
canonicalization logic.

Fix all unresolved CodeRabbit comments on this PR:

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

packages/app/e2e/sidebar/sidebar-session-links.spec.ts (1)

23-25: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Replace fixed sleep with observable URL stabilization.

Line 24 uses page.waitForTimeout(100), which is brittle and can still race under slower CI conditions.

Suggested change

     await expect(page).toHaveURL(new RegExp(`/${slug}/session/${two.id}(?:\\?|#|$)`))
-    await page.waitForTimeout(100)
-    await expect(page).toHaveURL(new RegExp(`/${slug}/session/${two.id}(?:\\?|#|$)`))
+    await expect
+      .poll(() => page.url(), { timeout: 1_000 })
+      .toMatch(new RegExp(`/${slug}/session/${two.id}(?:\\?|#|$)`))

As per coding guidelines: "Never use wall-clock waits like page.waitForTimeout(...) to make a test pass" and "Wait on observable state with expect(...), expect.poll(...), or existing helpers instead of assuming work is finished after an action".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/app/e2e/sidebar/sidebar-session-links.spec.ts` around lines 23 - 25,
The test uses a brittle fixed sleep (page.waitForTimeout(100)) between two URL
asserts; remove that sleep and instead wait on an observable state — replace the
pair of expects and the sleep with a single retryable wait such as using
expect(page).toHaveURL(new RegExp(`/${slug}/session/${two.id}(?:\\?|#|$)`), {
timeout: ... }) or use expect.poll to assert the page.url() stabilizes to the
same RegExp, ensuring you reference the existing identifiers slug and two.id and
remove page.waitForTimeout.

Fix all unresolved CodeRabbit comments on this PR:

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

packages/app/e2e/sidebar/sidebar-session-links.spec.ts (1)

37-45: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Avoid interpolating slug directly into the route regexp.

If slug ever contains regex metacharacters or a non-canonical path form, this assertion can become brittle or match the wrong route. Please verify it is already escaped/canonicalized, or switch to a shared route helper.

Verification script

#!/bin/bash
rg -n -C3 '\bslug\b|escapeRegExp|waitSession\(' packages/app/e2e packages/app/src

Expected: confirm whether the fixture already normalizes slug or whether an existing route helper should be used here instead of a raw RegExp.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/app/e2e/sidebar/sidebar-session-links.spec.ts` around lines 37 - 45,
The test builds a RegExp for selectedSessionUrl by interpolating slug directly
which can break if slug contains regex metacharacters; update the construction
used in selectedSessionUrl (and the subsequent expect(page).toHaveURL call and
expectUrlToStayMatched usage) to use a canonicalized/escaped slug or a shared
route helper instead of raw interpolation—ensure you either escape slug (e.g.,
via an escapeRegExp utility) before embedding it in the RegExp or replace the
RegExp construction with a route helper that returns the correct URL pattern for
the session route.

Fix all unresolved CodeRabbit comments on this PR:

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