Skip to content

fix(app): keep session tab preview from intercepting clicks - #442

Merged
shuv1337 merged 2 commits into
integration-v2from
fm/sc-i438-tab-popover
Oct 6, 2026
Merged

shuv1337 merged 2 commits into
integration-v2from
fm/sc-i438-tab-popover

Conversation

@shuv1337

@shuv1337 shuv1337 commented Oct 6, 2026

Copy link
Copy Markdown
Owner

Intent

Fix issue #438 in shuv1337/shuvcode. Main lead authorization corr=91920f991fadc644: "The session-tab hover popover stays open with inline pointer-events: auto after a folder switch and covers the Open file button. Close or neutralize the popover on switch so the click lands, and un-fixme the tabs.spec.ts test as the regression proof. This is product-facing, so mode no-mistakes, yolo on. Keep it to that bug under the captain's no-extra-gates rule: the existing e2e plus CI is the proof."

Existing evidence from the completed Cursor review on #439 (comment): in job https://github.com/shuv1337/shuvcode/actions/runs/37427591275/job/112150819534 the first Open file click succeeds, but the next open("notes.txt") after switching to a session in another folder fails on all three attempts. With the review panel open at 1440x900, the focused/switched session tab leaves its hover preview over the button. The trace shows data-component=session-tab-popover positioned at translate3d(553px, 43px, 0px) with inline pointer-events: auto, overriding tab-popover.css pointer-events: none; its FolderSwitch project span intercepts the click. The quarantine in merged PR #439 is temporary and must be removed with this fix.

Captain's current process constraints: no additional validation/live-instance gates beyond the ask and brief. Existing e2e plus CI is the proof. Review findings after green CI get a fix commit, a PR reply per finding, and CI only, never a second full no-mistakes run. Unrelated flaky tests are quarantined with tracking issues, never re-run or blocking.

What Changed

  • In tab-popover.css, the session-tab hover preview now uses pointer-events: none !important. Kobalte's layer stack sets an inline pointer-events: auto on every mounted layer. Without !important, that inline value won. A preview left open after a folder switch could then cover controls such as the review panel's Open file button and take their clicks.
  • Removed the temporary test.fixme quarantine (issue app e2e: FolderSwitch project label covers Open file button in tabs.spec folder-switch test #438) from the folder-switch regression test in e2e/regression/tabs.spec.ts, so it runs again as the regression check.

Fixes #438

Risk Assessment

✅ Low: The change is a single CSS change. pointer-events: none !important overrides the inline pointer-events: auto that Kobalte's layer stack sets on the popover, so the purely presentational popover can no longer catch clicks on what it covers. The only other change removes the temporary quarantine (test.fixme) from the regression e2e test, as the intent requires.

Testing

I built and served the app the way CI does (Vite build + preview with test fixtures) and drove it in Chromium. With the fix, the un-quarantined FolderSwitch test passed 6 of 6 runs. Without the fix it also passed locally, so the quarantined test alone doesn't reproduce the CI failure here. To check the bug itself, I wrote a throwaway probe that pins the open preview over the Open file button. Without the fix, the preview had computed pointer-events: auto, was the element under the button, and Playwright's click timed out because the preview header intercepted it. With the fix, inline pointer-events was still auto but the computed value was none, the click reached the button, and notes.txt opened. Screenshots are in the evidence directory. All --trace on runs timed out, including a run of an unrelated test, so that's a local tracing problem and not part of this result. The probe spec, build output and reports were removed from the worktree afterwards.

  • Live validation: ✅ go - 4 of 4 scenarios driven live against the product
Scenario Result Live Evidence
After switching to a session in another folder, the user clicks Open file while that tab's hover preview covers the button, and the click opens the file picker (notes.txt opens) ✅ pass live probe-after.log + probe-after/*.png: computed pointer-events none, elementFromPoint is the button, real click succeeds, contents:notes.txt visible
Adversarial: the same pinned preview on the base CSS blocks the click (shows the bug and that the probe catches it) ✅ pass live probe-before.log: computed pointer-events auto, hit=popover, click timed out with 'data-slot=header … intercepts pointer events'
Un-quarantined tabs.spec FolderSwitch test: open a file in each of three sessions across two folders, then switch back and forth; each tab keeps its own file ✅ pass live npx playwright test e2e/regression/tabs.spec.ts -g "each session tab shows its own file tab" --repeat-each=5 on the production build: 5 passed, plus 1 earlier single pass
The hover preview still appears with project, title and path when hovering a session tab ✅ pass live probe-after/popover-over-open-file.png shows the preview with FolderSwitch / Folder switch other / C:/OpenCode/FolderOther

Fixed build: hover preview pinned over the Open file button (computed pointer-events none)
Fixed build: Open file click went through the preview and notes.txt opened
Base CSS: preview covering the button that blocked the click

Evidence: Probe log, base CSS (reproduces the bug)

PROBE {"inline":"auto","computed":"auto","covers":true,"hit":"popover"} TimeoutError: locator.click: Timeout 5000ms exceeded. - <div data-slot="header">…</div> from <div>…</div> subtree intercepts pointer events


Running 1 test using 1 worker

PROBE {"inline":"auto","computed":"auto","covers":true,"hit":"popover"}
  ✘  1 [chromium] › e2e/regression/zz-popover-probe.spec.ts:10:1 › open session-tab preview does not intercept the Open file click it covers (6.5s)


  1) [chromium] › e2e/regression/zz-popover-probe.spec.ts:10:1 › open session-tab preview does not intercept the Open file click it covers 

    TimeoutError: locator.click: Timeout 5000ms exceeded.
    Call log:
    �[2m  - waiting for locator('#review-panel').getByRole('button', { name: 'Open file' })�[22m
    �[2m    - locator resolved to <button type="button" data-size="large" aria-label="Open file" data-variant="ghost-muted" data-component="icon-button-v2">…</button>�[22m
    �[2m  - attempting click action�[22m
    �[2m    2 × waiting for element to be visible, enabled and stable�[22m
    �[2m      - element is visible, enabled and stable�[22m
    �[2m      - scrolling into view if needed�[22m
    �[2m      - done scrolling�[22m
    �[2m      - <div data-slot="header">…</div> from <div>…</div> subtree intercepts pointer events�[22m
    �[2m    - retrying click action�[22m
    �[2m    - waiting 20ms�[22m
    �[2m    2 × waiting for element to be visible, enabled and stable�[22m
    �[2m      - element is visible, enabled and stable�[22m
    �[2m      - scrolling into view if needed�[22m
    �[2m      - done scrolling�[22m
    �[2m      - <div data-slot="header">…</div> from <div>…</div> subtree intercepts pointer events�[22m
    �[2m    - retrying click action�[22m
    �[2m      - waiting 100ms�[22m
    �[2m    10 × waiting for element to be visible, enabled and stable�[22m
    �[2m       - element is visible, enabled and stable�[22m
    �[2m       - scrolling into view if needed�[22m
    �[2m       - done scrolling�[22m
    �[2m       - <div data-slot="header">…</div> from <div>…</div> subtree intercepts pointer events�[22m
    �[2m     - retrying click action�[22m
    �[2m       - waiting 500ms�[22m


      48 |   expect(state.covers).toBe(true)
      49 |   // A real click: Playwright's actionability check fails if another element intercepts it.
    > 50 |   await button.click({ timeout: 5_000 })
         |                ^
      51 |   await panel.getByRole("button", { name: "notes.txt", exact: true }).click()
      52 |   await expect(panel.getByText("contents:notes.txt", { exact: true })).toBeVisible()
      53 |   await page.screenshot({ path: `${EVIDENCE}/after-open-file-click.png` })
        at ~/.no-mistakes/worktrees/284027b67ce8/01M4858S28EFD5PWY56W1S6BQ3/packages/app/e2e/regression/zz-popover-probe.spec.ts:50:16

    attachment #1: screenshot (image/png) ──────────────────────────────────────────────────────────
    ../../../../../../../../tmp/pw-probe-before/regression-zz-popover-prob-a197a-e-Open-file-click-it-covers-chromium/test-failed-1.png
    ────────────────────────────────────────────────────────────────────────────────────────────────

    attachment #2: video (video/webm) ──────────────────────────────────────────────────────────────
    ../../../../../../../../tmp/pw-probe-before/regression-zz-popover-prob-a197a-e-Open-file-click-it-covers-chromium/video.webm
    ────────────────────────────────────────────────────────────────────────────────────────────────

    Error Context: ../../../../../../../../tmp/pw-probe-before/regression-zz-popover-prob-a197a-e-Open-file-click-it-covers-chromium/error-context.md

  1 failed
    [chromium] › e2e/regression/zz-popover-probe.spec.ts:10:1 › open session-tab preview does not intercept the Open file click it covers 
Evidence: Probe log, fixed CSS

PROBE {"inline":"auto","computed":"none","covers":true,"hit":""} ✓ open session-tab preview does not intercept the Open file click it covers


Running 1 test using 1 worker

PROBE {"inline":"auto","computed":"none","covers":true,"hit":""}
  ✓  1 [chromium] › e2e/regression/zz-popover-probe.spec.ts:10:1 › open session-tab preview does not intercept the Open file click it covers (1.7s)

  1 passed (9.3s)
Evidence: Un-quarantined test with base CSS (passes locally; CI timing not reproduced)

Running 3 tests using 3 workers

  ✓  1 [chromium] › e2e/regression/tabs.spec.ts:486:1 › each session tab shows its own file tab after a switch to a session in another folder (2.0s)
  ✓  2 [chromium] › e2e/regression/tabs.spec.ts:486:1 › each session tab shows its own file tab after a switch to a session in another folder (2.0s)
  ✓  3 [chromium] › e2e/regression/tabs.spec.ts:486:1 › each session tab shows its own file tab after a switch to a session in another folder (2.0s)

  3 passed (10.1s)
- Outcome: ⚠️ 1 info across 1 run (6m26s)

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

✅ **Review** - passed

✅ No issues found.

⚠️ **Test** - 1 info
  • ℹ️ packages/app/e2e/regression/tabs.spec.ts:486 - The un-quarantined FolderSwitch test also passes locally on the base CSS (3/3), so on this machine it doesn't fail before the fix. It depends on CI timing to open the hover preview. A deterministic probe pinning the preview over the button did reproduce the bug and confirm the fix. The intent names the existing e2e plus CI as the proof, so no extra test was added.
  • Live validation: ✅ go - 4 of 4 scenarios driven live against the product
Scenario Result Live Evidence
After switching to a session in another folder, the user clicks Open file while that tab's hover preview covers the button, and the click opens the file picker (notes.txt opens) ✅ pass live probe-after.log + probe-after/*.png: computed pointer-events none, elementFromPoint is the button, real click succeeds, contents:notes.txt visible
Adversarial: the same pinned preview on the base CSS blocks the click (shows the bug and that the probe catches it) ✅ pass live probe-before.log: computed pointer-events auto, hit=popover, click timed out with 'data-slot=header … intercepts pointer events'
Un-quarantined tabs.spec FolderSwitch test: open a file in each of three sessions across two folders, then switch back and forth; each tab keeps its own file ✅ pass live npx playwright test e2e/regression/tabs.spec.ts -g &#34;each session tab shows its own file tab&#34; --repeat-each=5 on the production build: 5 passed, plus 1 earlier single pass
The hover preview still appears with project, title and path when hovering a session tab ✅ pass live probe-after/popover-over-open-file.png shows the preview with FolderSwitch / Folder switch other / C:/OpenCode/FolderOther
  • PLAYWRIGHT_BUILD=1 PLAYWRIGHT_PORT=3917 PLAYWRIGHT_SERVER_PORT=3917 npx playwright test e2e/regression/tabs.spec.ts -g &#34;each session tab shows its own file tab&#34; (un-quarantined test, 1 run and --repeat-each=5, fix applied)
  • Same test with base-commit tab-popover.css restored (--repeat-each=3): it also passes locally, so it doesn't reproduce the CI timing failure on this machine
  • Temporary probe spec (deleted afterwards) at 1440x900: switch to the session in the other folder, hover its tab to open the preview, move the preview over the Open file button as in the CI trace, record inline vs. computed pointer-events and elementFromPoint, take a screenshot, then do a real Playwright click and open notes.txt
  • Ran the same probe with the base-commit CSS restored to reproduce the bug
  • Ran another tabs.spec test with --trace on and it timed out the same way, confirming those timeouts come from the trace harness, not the change
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

Kobalte's layer stack sets inline pointer-events: auto on every mounted
layer, overriding the preview's pointer-events: none. After a folder switch
the open preview covered the review panel's Open file button. Force the
non-interactive preview and restore the quarantined e2e regression.

Closes #438

shuv1337 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

🔍 Automated Cursor review is underway. Please hold off on merging until the review comment lands here (usually 10–20 minutes).

@cursor

cursor Bot commented Oct 6, 2026

Copy link
Copy Markdown

Review complete:

Low — packages/app/e2e/regression/tabs.spec.ts:486
Removing test.fixme does not lock this fix. The test clicks Open file without opening [data-component="session-tab-popover"]. It fails only when the pointer rests on the tab long enough for the 750ms open delay to leave that preview over the button. On the pre-fix CSS it passes when that race does not happen, so reverting pointer-events: none !important in packages/app/src/shell/titlebar/tab-popover.css:26 can stay green.
Suggested fix: in this test, hover the switched session tab until the preview is open and covering Open file, then click Open file and assert the chosen file opens.

No High or Medium findings. The !important rule is only on this preview. Its contents are non-interactive text (tab-popover.tsx), hover-to-keep-open is already disabled (ignoreSafeArea, closeDelay 0), and Kobalte’s outside dismiss listens on document rather than requiring the layer to be the hit target. The popper positioner is already pointer-events: none; the inline pointer-events: auto that was winning is on this same content node.

@shuv1337

shuv1337 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Re Cursor Low (tabs.spec.ts:486, test does not lock the fix): fixed in 5b2723e. After switching to the other-folder session, the test now waits for [data-component="session-tab-popover"] to be visible before clicking Open file. Locally, with the pre-fix CSS it fails with the FolderSwitch project span intercepting the click; with the fix it passes.

@shuv1337
shuv1337 merged commit a8a159a into integration-v2 Oct 6, 2026
3 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.

app e2e: FolderSwitch project label covers Open file button in tabs.spec folder-switch test

1 participant