From 8d5e7ffe1c74bd222d686f226aeae77c1f76c32d Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Mon, 27 Jul 2026 09:40:04 -0400 Subject: [PATCH 01/11] fix(store): adds scoped Zod validation to updateViewState --- pnpm-workspace.yaml | 5 +++ src/app/stores/view.ts | 23 +++++++++++--- tests/stores/view.test.ts | 64 ++++++++++++++++++++++++++++++++++++++- 3 files changed, 86 insertions(+), 6 deletions(-) diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index 9b36f203..83120415 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -1,4 +1,9 @@ packages: - "mcp" +allowBuilds: + esbuild: true + sharp: true + workerd: true + strictDepBuilds: false diff --git a/src/app/stores/view.ts b/src/app/stores/view.ts index 0c94a14a..b04cabf1 100644 --- a/src/app/stores/view.ts +++ b/src/app/stores/view.ts @@ -224,12 +224,25 @@ export function resetViewState(): void { ); } +// KNOWN LIMITATION: ViewStateSchema.pick() only prevents inflation of TOP-LEVEL fields +// absent from `partial`. It does NOT protect a nested field's own missing sub-keys from +// being backfilled with schema defaults if a caller ever passes a partially-populated +// nested object (e.g. `{ globalFilter: { org: "x" } }` without `repo`) — Zod's per-field +// `.default()` still fires within a nested object's own schema regardless of `.pick()` at +// the parent level. Not a regression: no current call site passes anything but flat scalar +// fields, and `updateConfig()` in config.ts has this same limitation today. export function updateViewState(partial: Partial): void { - setViewState( - produce((draft) => { - Object.assign(draft, partial); - }) - ); + const keys = Object.keys(partial) as (keyof ViewState)[]; + if (keys.length === 0) return; + const shape = Object.fromEntries(keys.map((k) => [k, true])) as Partial>; + const validated = ViewStateSchema.pick(shape).safeParse(partial); + if (!validated.success) { + pushNotification("view:updateViewState", "Some view settings could not be saved due to invalid data.", "warning"); + return; + } + setViewState(produce((draft) => { + Object.assign(draft, validated.data); + })); } export function ignoreItem(item: IgnoredItem): void { diff --git a/tests/stores/view.test.ts b/tests/stores/view.test.ts index a1981a09..ba5710e0 100644 --- a/tests/stores/view.test.ts +++ b/tests/stores/view.test.ts @@ -28,7 +28,8 @@ import { untrackJiraItem, moveJiraItem, } from "../../src/app/stores/view"; -import type { IgnoredItem, TrackedItem } from "../../src/app/stores/view"; +import type { IgnoredItem, TrackedItem, ViewState } from "../../src/app/stores/view"; +import { getNotifications, clearNotifications } from "../../src/app/lib/errors"; // view.ts uses createStore — setters work outside reactive context. // We use createRoot only for initViewPersistence (which calls createEffect). @@ -79,6 +80,67 @@ describe("updateViewState", () => { expect(viewState.globalFilter).toEqual({ org: null, repo: null }); expect(viewState.ignoredItems).toEqual([]); }); + + it("is a safe no-op when called with an empty object", () => { + updateViewState({ lastActiveTab: "pullRequests" }); + updateViewState({}); + expect(viewState.lastActiveTab).toBe("pullRequests"); + }); + + // Real call sites (DashboardPage, SettingsPage, config.ts, IssuesTab) only ever pass + // these two field/type shapes — verify both validate and apply correctly. + it("applies a real call-site shape: { lastActiveTab: string }", () => { + updateViewState({ lastActiveTab: "jiraAssigned" }); + expect(viewState.lastActiveTab).toBe("jiraAssigned"); + }); + + it("applies a real call-site shape: { hideDepDashboard: boolean }", () => { + expect(viewState.hideDepDashboard).toBe(true); + updateViewState({ hideDepDashboard: false }); + expect(viewState.hideDepDashboard).toBe(false); + updateViewState({ hideDepDashboard: true }); + expect(viewState.hideDepDashboard).toBe(true); + }); + + // Structural guard: updating ANY single field must never wipe other fields. + // Catches Zod v4 .partial().safeParse() default inflation (BUG-001 class, mirrors config.test.ts). + it.each([ + ["lastActiveTab", { lastActiveTab: "actions" }], + ["showPrRuns", { showPrRuns: true }], + ["hideDepDashboard", { hideDepDashboard: false }], + ["dependencyExpandedGroups", { dependencyExpandedGroups: ["patch"] }], + ] satisfies [string, Partial][])("updating only %s preserves other non-default fields", (_fieldName, patch) => { + // Seed an unrelated field with a non-default value first. + updateViewState({ lastActiveTab: "pullRequests" }); + expect(viewState.lastActiveTab).toBe("pullRequests"); + + // Now update a (possibly different) single field. + updateViewState(patch); + + // The previously-seeded field must still hold its non-default value + // unless this iteration's patch targeted it directly. + if (!("lastActiveTab" in patch)) { + expect(viewState.lastActiveTab).toBe("pullRequests"); + } + }); + + describe("invalid data", () => { + beforeEach(() => { + clearNotifications(); + }); + + it("rejects a single field with the wrong type, pushes a warning notification, and leaves the store unchanged", () => { + const before = viewState.hideDepDashboard; + updateViewState({ hideDepDashboard: "not-a-boolean" as unknown as boolean }); + + expect(viewState.hideDepDashboard).toBe(before); + + const notifications = getNotifications(); + expect(notifications).toHaveLength(1); + expect(notifications[0].source).toBe("view:updateViewState"); + expect(notifications[0].severity).toBe("warning"); + }); + }); }); describe("setGlobalFilter", () => { From 448c16cef1c440fe1cb050ad5a389e4e8964b165 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Mon, 27 Jul 2026 09:41:08 -0400 Subject: [PATCH 02/11] refactor(ui): migrates IgnoreBadge to Kobalte Popover --- src/app/components/dashboard/IgnoreBadge.tsx | 38 ++++++-------------- tests/components/IgnoreBadge.test.tsx | 16 +++++---- 2 files changed, 20 insertions(+), 34 deletions(-) diff --git a/src/app/components/dashboard/IgnoreBadge.tsx b/src/app/components/dashboard/IgnoreBadge.tsx index 48b16d06..e4067eeb 100644 --- a/src/app/components/dashboard/IgnoreBadge.tsx +++ b/src/app/components/dashboard/IgnoreBadge.tsx @@ -1,4 +1,5 @@ import { createSignal, For, Show } from "solid-js"; +import { Popover } from "@kobalte/core/popover"; import type { IgnoredItem } from "../../stores/view"; import { Tooltip } from "../shared/Tooltip"; @@ -28,12 +29,6 @@ function formatDate(ts: number): string { export default function IgnoreBadge(props: IgnoreBadgeProps) { const [open, setOpen] = createSignal(false); - function handleBackdropClick(e: MouseEvent) { - if (e.target === e.currentTarget) { - setOpen(false); - } - } - function handleUnignoreAll() { for (const item of props.items) { props.onUnignore(item.id); @@ -43,13 +38,11 @@ export default function IgnoreBadge(props: IgnoreBadgeProps) { return ( 0}> -
+ - + - - {/* Backdrop */} - + + + ); } diff --git a/tests/components/IgnoreBadge.test.tsx b/tests/components/IgnoreBadge.test.tsx index ba534eea..fe784243 100644 --- a/tests/components/IgnoreBadge.test.tsx +++ b/tests/components/IgnoreBadge.test.tsx @@ -1,5 +1,5 @@ import { describe, it, expect, vi } from "vitest"; -import { render, screen } from "@solidjs/testing-library"; +import { render, screen, fireEvent } from "@solidjs/testing-library"; import userEvent from "@testing-library/user-event"; import IgnoreBadge from "../../src/app/components/dashboard/IgnoreBadge"; import type { IgnoredItem } from "../../src/app/stores/view"; @@ -114,7 +114,7 @@ describe("IgnoreBadge", () => { expect(onUnignore).toHaveBeenCalledWith(3); }); - it("clicking backdrop closes popover", async () => { + it("clicking outside closes popover", async () => { const user = userEvent.setup(); const items = [makeIgnoredItem()]; render(() => {}} />); @@ -122,11 +122,13 @@ describe("IgnoreBadge", () => { await user.click(button); expect(button.getAttribute("aria-expanded")).toBe("true"); - // The backdrop is aria-hidden div with fixed positioning - const backdrop = document.querySelector('[aria-hidden="true"]') as HTMLElement; - expect(backdrop).toBeDefined(); - // Simulate clicking the backdrop itself (target === currentTarget) - await user.click(backdrop); + // Kobalte's createInteractOutside registers its document pointerdown listener + // inside a setTimeout(0) (to avoid catching the click that opened the popover). + // This file uses userEvent's real-timer async model throughout, so wait a real + // tick rather than introducing fake timers. + await new Promise((resolve) => setTimeout(resolve, 0)); + fireEvent.pointerDown(document.body); + expect(button.getAttribute("aria-expanded")).toBe("false"); }); }); From e31f3b5d7df557348cb395b5b3e14f3875d1adea Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Mon, 27 Jul 2026 09:41:33 -0400 Subject: [PATCH 03/11] feat(ui): adds tooltip to scope filter label --- src/app/components/shared/ScopeToggle.tsx | 10 +++++--- tests/components/shared/ScopeToggle.test.tsx | 25 +++++++++++++++++++- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/src/app/components/shared/ScopeToggle.tsx b/src/app/components/shared/ScopeToggle.tsx index 2c31126b..01e7b507 100644 --- a/src/app/components/shared/ScopeToggle.tsx +++ b/src/app/components/shared/ScopeToggle.tsx @@ -1,3 +1,5 @@ +import { Tooltip } from "./Tooltip"; + interface ScopeToggleProps { value: string; onChange: (field: string, value: string) => void; @@ -17,9 +19,11 @@ export default function ScopeToggle(props: ScopeToggleProps) { props.onChange("scope", e.currentTarget.checked ? "involves_me" : "all") } /> - - {checked() ? "Involves me" : "All activity"} - + + + {checked() ? "Involves me" : "All activity"} + + ); } diff --git a/tests/components/shared/ScopeToggle.test.tsx b/tests/components/shared/ScopeToggle.test.tsx index ecb48581..3ee5b150 100644 --- a/tests/components/shared/ScopeToggle.test.tsx +++ b/tests/components/shared/ScopeToggle.test.tsx @@ -1,8 +1,16 @@ -import { describe, it, expect, vi } from "vitest"; +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { render, screen, fireEvent } from "@solidjs/testing-library"; import ScopeToggle from "../../../src/app/components/shared/ScopeToggle"; describe("ScopeToggle", () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + it("renders checkbox checked when value is 'involves_me'", () => { render(() => {}} />); const checkbox = screen.getByRole("checkbox", { name: /Scope filter/i }); @@ -46,4 +54,19 @@ describe("ScopeToggle", () => { const checkbox = screen.getByRole("checkbox", { name: "Scope filter" }); expect(checkbox).toBeDefined(); }); + + it("shows a tooltip explaining the toggle after hovering the label", () => { + const { container } = render(() => ( + {}} /> + )); + const trigger = container.querySelector("span.inline-flex")!; + expect(document.body.textContent).not.toContain( + "Toggle between items involving you and all tracked activity" + ); + fireEvent.pointerEnter(trigger); + vi.advanceTimersByTime(300); + expect(document.body.textContent).toContain( + "Toggle between items involving you and all tracked activity" + ); + }); }); From a01f7aafba7039b6a725ab0c3b01c6af89c0b030 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Mon, 27 Jul 2026 09:48:15 -0400 Subject: [PATCH 04/11] test(e2e): covers notification drawer animation and reduced-motion behavior --- e2e/notifications.spec.ts | 100 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) create mode 100644 e2e/notifications.spec.ts diff --git a/e2e/notifications.spec.ts b/e2e/notifications.spec.ts new file mode 100644 index 00000000..eb5caf84 --- /dev/null +++ b/e2e/notifications.spec.ts @@ -0,0 +1,100 @@ +import { test, expect, type Page } from "@playwright/test"; +import { setupAuth } from "./helpers"; + +const emptyGraphqlFixture = { + data: { + issues: { issueCount: 0, pageInfo: { hasNextPage: false, endCursor: null }, nodes: [] }, + prInvolves: { issueCount: 0, pageInfo: { hasNextPage: false, endCursor: null }, nodes: [] }, + prReviewReq: { issueCount: 0, pageInfo: { hasNextPage: false, endCursor: null }, nodes: [] }, + rateLimit: { limit: 5000, remaining: 4999, resetAt: "2099-01-01T00:00:00Z" }, + }, +}; + +/** + * Fakes a real GitHub primary-rate-limit response on the FIRST /graphql POST so + * @octokit/plugin-throttling's onRateLimit handler fires (github.ts:86-98), which calls + * pushNotification("rate-limit", ..., "warning", true). The plugin then auto-retries + * once internally (retryCount < 1 => true); the SECOND POST returns normal fixture data + * so the dashboard loads successfully afterward. + */ +async function triggerRateLimitOnFirstGraphqlCall(page: Page) { + let callCount = 0; + await page.route("https://api.github.com/graphql", (route) => { + callCount += 1; + if (callCount === 1) { + const resetEpochSeconds = Math.floor(Date.now() / 1000) + 1; + return route.fulfill({ + status: 403, + headers: { + "x-ratelimit-limit": "5000", + "x-ratelimit-remaining": "0", + "x-ratelimit-reset": String(resetEpochSeconds), + // Cross-origin fetch() only exposes safelisted response headers to JS unless the + // server opts in via Access-Control-Expose-Headers (the real GitHub API does this) — + // without it, @octokit/plugin-throttling can't see x-ratelimit-remaining and never + // fires onRateLimit; the request just fails and surfaces as a generic api error instead. + "access-control-expose-headers": "x-ratelimit-limit, x-ratelimit-remaining, x-ratelimit-reset", + }, + json: { message: "API rate limit exceeded for token." }, + }); + } + return route.fulfill({ status: 200, json: emptyGraphqlFixture }); + }); +} + +async function openDrawerFromBell(page: Page) { + await page.goto("/dashboard"); + + const bell = page.getByRole("button", { name: /Notifications, \d+ unread/i }); + await expect(bell).toBeVisible({ timeout: 15_000 }); + await bell.click(); + + const overlay = page.getByTestId("notification-overlay"); + await expect(overlay).toBeVisible(); + const dialog = page.getByRole("dialog"); + await expect(dialog).toBeVisible(); + // ToastContainer renders the same notification as a toast concurrently (expected, + // per Task 7 plan) — scope to the dialog to avoid a strict-mode double match. + await expect(dialog.getByText(/Rate limit hit/i)).toBeVisible(); + return overlay; +} + +// ── Open/close animation via real rate-limit-triggered notification ───────── + +test("rate-limit notification opens the drawer and Escape closes it", async ({ page }) => { + await setupAuth(page); + await triggerRateLimitOnFirstGraphqlCall(page); + + const overlay = await openDrawerFromBell(page); + + await page.keyboard.press("Escape"); + await expect(overlay).toBeHidden(); +}); + +test("rate-limit notification opens the drawer and the close button closes it", async ({ page }) => { + await setupAuth(page); + await triggerRateLimitOnFirstGraphqlCall(page); + + const overlay = await openDrawerFromBell(page); + + await page.getByLabel("Close notifications").click(); + await expect(overlay).toBeHidden(); +}); + +// ── prefers-reduced-motion ─────────────────────────────────────────────────── + +test("drawer still opens and closes correctly under prefers-reduced-motion", async ({ page }) => { + await page.emulateMedia({ reducedMotion: "reduce" }); + await setupAuth(page); + await triggerRateLimitOnFirstGraphqlCall(page); + + const overlay = await openDrawerFromBell(page); + + // Under prefers-reduced-motion, index.css disables the drawer keyframe animation + // entirely (`.drawer-overlay[data-expanded] { animation: none }`), unlike the + // normal `overlay-fade-in` animation applied without the media query match. + await expect(overlay).toHaveCSS("animation-name", "none"); + + await page.getByLabel("Close notifications").click(); + await expect(overlay).toBeHidden(); +}); From d22b421628a1996bfc3224f265028fe428aa6d81 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Mon, 27 Jul 2026 09:50:52 -0400 Subject: [PATCH 05/11] refactor(settings): migrates 3 native selects to Kobalte Select --- src/app/components/settings/SettingsPage.tsx | 123 +++++++++++++----- .../components/settings/SettingsPage.test.tsx | 44 ++++--- .../settings/actions-toggle.test.tsx | 52 +++----- 3 files changed, 134 insertions(+), 85 deletions(-) diff --git a/src/app/components/settings/SettingsPage.tsx b/src/app/components/settings/SettingsPage.tsx index e7d3cd8c..77e6dee7 100644 --- a/src/app/components/settings/SettingsPage.tsx +++ b/src/app/components/settings/SettingsPage.tsx @@ -1,9 +1,9 @@ import { createSignal, createMemo, Show, For, onCleanup, onMount } from "solid-js"; +import { Select } from "@kobalte/core/select"; import * as Sentry from "@sentry/solid"; import { getRelayStatus } from "../../lib/mcp-relay"; import { useNavigate } from "@solidjs/router"; import { config, updateConfig, updateJiraConfig, updateJiraCustomFields, updateJiraCustomScopes, setMonitoredRepo, isActionsBasedTab } from "../../stores/config"; -import type { Config } from "../../stores/config"; import { viewState, updateViewState } from "../../stores/view"; import { clearAuth, jiraAuth, setJiraAuth, clearJiraConfigFull, isJiraAuthenticated, token, setAuthFromPat } from "../../stores/auth"; import type { GitHubUser } from "../../stores/auth"; @@ -34,6 +34,16 @@ import type { RepoRef } from "../../services/api"; const VALID_JIRA_CLIENT_ID_RE = /^[A-Za-z0-9_-]+$/; +interface NumberSelectOption { + value: number; + label: string; +} + +interface StringSelectOption { + value: string; + label: string; +} + export const SETTINGS_PAGE_SECTION_IDS = [ "orgs-repos", "tracked-users", @@ -427,7 +437,7 @@ export default function SettingsPage() { // ── Refresh interval options ────────────────────────────────────────────── - const refreshOptions = [ + const refreshOptions: NumberSelectOption[] = [ { value: 60, label: "1 minute" }, { value: 120, label: "2 minutes" }, { value: 300, label: "5 minutes (default)" }, @@ -437,7 +447,7 @@ export default function SettingsPage() { { value: 0, label: "Off" }, ]; - const tabOptions = createMemo(() => [ + const tabOptions = createMemo(() => [ { value: "issues", label: "Issues" }, { value: "pullRequests", label: "Pull Requests" }, ...(config.enableActions ? [{ value: "actions", label: "GitHub Actions" }] : []), @@ -448,7 +458,7 @@ export default function SettingsPage() { ]); - const itemsPerPageOptions = [ + const itemsPerPageOptions: NumberSelectOption[] = [ { value: 10, label: "10" }, { value: 25, label: "25" }, { value: 50, label: "50" }, @@ -608,17 +618,32 @@ export default function SettingsPage() { label="Refresh interval" description="How often to poll GitHub for new data" > - opt.value === config.refreshInterval) ?? null} + onChange={(opt) => opt && saveWithFeedback({ refreshInterval: opt.value })} + itemComponent={(itemProps) => ( + + {itemProps.item.rawValue.label} + + )} > - {refreshOptions.map((opt) => ( - - ))} - + + > + {(state) => state.selectedOption()?.label ?? ""} + + + + + + + + - opt.value === config.itemsPerPage) ?? null} + onChange={(opt) => opt && saveWithFeedback({ itemsPerPage: opt.value })} + itemComponent={(itemProps) => ( + + {itemProps.item.rawValue.label} + + )} > - {itemsPerPageOptions.map((opt) => ( - - ))} - + + > + {(state) => state.selectedOption()?.label ?? ""} + + + + + + + + @@ -742,17 +782,32 @@ export default function SettingsPage() { label="Default tab" description="Tab shown when opening the dashboard fresh" > - opt.value === config.defaultTab) ?? null} + onChange={(opt) => opt && saveWithFeedback({ defaultTab: opt.value })} + itemComponent={(itemProps) => ( + + {itemProps.item.rawValue.label} + + )} > - {tabOptions().map((opt) => ( - - ))} - + + > + {(state) => state.selectedOption()?.label ?? ""} + + + + + + + + { describe("SettingsPage — Refresh interval", () => { it("shows current refresh interval value", () => { renderSettings(); - screen.getByDisplayValue("5 minutes (default)"); + screen.getByRole("button", { name: "5 minutes (default)" }); }); it("changing refresh interval calls updateConfig", async () => { const user = userEvent.setup(); renderSettings(); - const select = screen.getByDisplayValue("5 minutes (default)"); - await user.selectOptions(select, "60"); + const trigger = screen.getByRole("button", { name: "5 minutes (default)" }); + await user.click(trigger); + await user.click(screen.getByRole("option", { name: "1 minute" })); expect(config.refreshInterval).toBe(60); }); }); @@ -261,14 +262,15 @@ describe("SettingsPage — Appearance", () => { it("shows current items per page value", () => { renderSettings(); - screen.getByDisplayValue("25"); + screen.getByRole("button", { name: "25" }); }); it("changing items per page updates config", async () => { const user = userEvent.setup(); renderSettings(); - const ippSelect = screen.getByDisplayValue("25"); - await user.selectOptions(ippSelect, "50"); + const trigger = screen.getByRole("button", { name: "25" }); + await user.click(trigger); + await user.click(screen.getByRole("option", { name: "50" })); expect(config.itemsPerPage).toBe(50); }); }); @@ -276,14 +278,15 @@ describe("SettingsPage — Appearance", () => { describe("SettingsPage — Tabs", () => { it("shows current default tab value", () => { renderSettings(); - screen.getByDisplayValue("Issues"); + screen.getByRole("button", { name: "Issues" }); }); it("changing default tab updates config", async () => { const user = userEvent.setup(); renderSettings(); - const tabSelect = screen.getByDisplayValue("Issues"); - await user.selectOptions(tabSelect, "pullRequests"); + const trigger = screen.getByRole("button", { name: "Issues" }); + await user.click(trigger); + await user.click(screen.getByRole("option", { name: "Pull Requests" })); expect(config.defaultTab).toBe("pullRequests"); }); @@ -978,15 +981,21 @@ describe("SettingsPage — enableTracking toggle", () => { expect(config.defaultTab).toBe("pullRequests"); }); - it("shows 'Tracked Items' option in defaultTab select when enableTracking is true", () => { + it("shows 'Tracked Items' option in defaultTab select when enableTracking is true", async () => { + const user = userEvent.setup(); updateConfig({ enableTracking: true }); renderSettings(); + await user.click(screen.getByRole("button", { name: "Issues" })); screen.getByRole("option", { name: "Tracked Items" }); }); - it("hides 'Tracked Items' option in defaultTab select when enableTracking is false", () => { + it("hides 'Tracked Items' option in defaultTab select when enableTracking is false", async () => { + const user = userEvent.setup(); updateConfig({ enableTracking: false }); renderSettings(); + // Open the trigger first — Kobalte doesn't mount role="option" elements until the + // listbox opens, so querying for absence without opening would pass vacuously. + await user.click(screen.getByRole("button", { name: "Issues" })); expect(screen.queryByRole("option", { name: "Tracked Items" })).toBeNull(); }); @@ -1027,7 +1036,8 @@ describe("SettingsPage — custom tabs in default tab dropdown", () => { updateConfig({ customTabs: [] }); }); - it("custom tab name appears as an option in the default tab select", () => { + it("custom tab name appears as an option in the default tab select", async () => { + const user = userEvent.setup(); updateConfig({ customTabs: [{ id: "custom-tab-1", @@ -1040,10 +1050,12 @@ describe("SettingsPage — custom tabs in default tab dropdown", () => { }], }); renderSettings(); + await user.click(screen.getByRole("button", { name: "Issues" })); screen.getByRole("option", { name: "My Custom Tab" }); }); - it("multiple custom tabs all appear in the default tab select", () => { + it("multiple custom tabs all appear in the default tab select", async () => { + const user = userEvent.setup(); updateConfig({ customTabs: [ { @@ -1067,6 +1079,7 @@ describe("SettingsPage — custom tabs in default tab dropdown", () => { ], }); renderSettings(); + await user.click(screen.getByRole("button", { name: "Issues" })); screen.getByRole("option", { name: "Alpha Tab" }); screen.getByRole("option", { name: "Beta Tab" }); }); @@ -1085,8 +1098,9 @@ describe("SettingsPage — custom tabs in default tab dropdown", () => { }], }); renderSettings(); - const tabSelect = screen.getByDisplayValue("Issues"); - await user.selectOptions(tabSelect, "my-tab"); + const trigger = screen.getByRole("button", { name: "Issues" }); + await user.click(trigger); + await user.click(screen.getByRole("option", { name: "My Tab" })); expect(config.defaultTab).toBe("my-tab"); }); }); diff --git a/tests/components/settings/actions-toggle.test.tsx b/tests/components/settings/actions-toggle.test.tsx index cefaae2c..4270bd46 100644 --- a/tests/components/settings/actions-toggle.test.tsx +++ b/tests/components/settings/actions-toggle.test.tsx @@ -1,5 +1,6 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { screen, fireEvent } from "@solidjs/testing-library"; +import userEvent from "@testing-library/user-event"; // ── localStorage mock ──────────────────────────────────────────────────────── @@ -277,38 +278,25 @@ describe("Actions toggle — negative cases", () => { }); describe("Actions toggle — default tab dropdown filtering", () => { - it("excludes GitHub Actions option from default tab select when enableActions is false", () => { + // The default tab select is a Kobalte Select — options only mount in the DOM once + // the trigger is opened, so each test must open it before asserting presence/absence. + it("excludes GitHub Actions option from default tab select when enableActions is false", async () => { + const user = userEvent.setup(); updateConfig({ enableActions: false }); renderSettings(); - const selects = document.querySelectorAll("select"); - let actionsOptionFound = false; - for (const sel of selects) { - for (const opt of sel.options) { - if (opt.value === "actions") { - actionsOptionFound = true; - break; - } - } - } - expect(actionsOptionFound).toBe(false); + await user.click(screen.getByRole("button", { name: "Issues" })); + expect(screen.queryByRole("option", { name: "GitHub Actions" })).toBeNull(); }); - it("includes GitHub Actions option in default tab select when enableActions is true", () => { + it("includes GitHub Actions option in default tab select when enableActions is true", async () => { + const user = userEvent.setup(); renderSettings(); - const selects = document.querySelectorAll("select"); - let actionsOptionFound = false; - for (const sel of selects) { - for (const opt of sel.options) { - if (opt.value === "actions") { - actionsOptionFound = true; - break; - } - } - } - expect(actionsOptionFound).toBe(true); + await user.click(screen.getByRole("button", { name: "Issues" })); + screen.getByRole("option", { name: "GitHub Actions" }); }); - it("excludes actions-based custom tabs from default tab select when enableActions is false", () => { + it("excludes actions-based custom tabs from default tab select when enableActions is false", async () => { + const user = userEvent.setup(); updateConfig({ enableActions: false, customTabs: [ @@ -317,16 +305,8 @@ describe("Actions toggle — default tab dropdown filtering", () => { ], }); renderSettings(); - const selects = document.querySelectorAll("select"); - let actionsCustomFound = false; - let issuesCustomFound = false; - for (const sel of selects) { - for (const opt of sel.options) { - if (opt.value === "my-actions") actionsCustomFound = true; - if (opt.value === "my-issues") issuesCustomFound = true; - } - } - expect(actionsCustomFound).toBe(false); - expect(issuesCustomFound).toBe(true); + await user.click(screen.getByRole("button", { name: "Issues" })); + expect(screen.queryByRole("option", { name: "My Actions" })).toBeNull(); + screen.getByRole("option", { name: "My Issues" }); }); }); From 68047ad0cd31ceb9671c269c9c99af138d273576 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Mon, 27 Jul 2026 10:02:18 -0400 Subject: [PATCH 06/11] test(e2e): isolates dependency bot-detection signals and covers abandoned-dependency pill rendering --- e2e/dependencies.spec.ts | 186 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 184 insertions(+), 2 deletions(-) diff --git a/e2e/dependencies.spec.ts b/e2e/dependencies.spec.ts index 421407fb..c5467c03 100644 --- a/e2e/dependencies.spec.ts +++ b/e2e/dependencies.spec.ts @@ -26,10 +26,10 @@ function makeDepPR(overrides: Record = {}) { }; } -function graphqlWithDepPRs(prs: Record[]) { +function graphqlWithDepPRs(prs: Record[], issues: Record[] = []) { return { data: { - issues: { issueCount: 0, pageInfo: { hasNextPage: false, endCursor: null }, nodes: [] }, + issues: { issueCount: issues.length, pageInfo: { hasNextPage: false, endCursor: null }, nodes: issues }, prInvolves: { issueCount: prs.length, pageInfo: { hasNextPage: false, endCursor: null }, @@ -41,6 +41,69 @@ function graphqlWithDepPRs(prs: Record[]) { }; } +function makeDashboardIssue(overrides: Record = {}) { + return { + id: "issue_dashboard_1", + databaseId: 5001, + number: 1, + title: "Dependency Dashboard", + state: "OPEN", + url: "https://github.com/testorg/testrepo/issues/1", + createdAt: recentDate, + updatedAt: recentDate, + author: { login: "renovate[bot]", avatarUrl: "https://avatars.githubusercontent.com/in/2740" }, + labels: { nodes: [] }, + assignees: { nodes: [] }, + comments: { totalCount: 0 }, + repository: { nameWithOwner: "testorg/testrepo", stargazerCount: 10 }, + ...overrides, + }; +} + +const ABANDONED_LODASH_BODY = `## Abandoned + +| Datasource | Package | Last Updated | +| --- | --- | --- | +| npm | lodash | 2025-01-01 | +`; + +/** + * Registers a single /graphql handler that serves the light combined search response for + * every call EXCEPT the dashboard-issue-body follow-up query, which it detects by inspecting + * the GraphQL query text itself — not `variables.request?.apiSource`. That field is consumed + * internally by @octokit/graphql as reserved request-config (used only for the app's own + * analytics hook, see github.ts) and never reaches the serialized wire body. Both + * DASHBOARD_ISSUE_BODIES_QUERY and DEP_PR_BODIES_QUERY (and the heavy PR backfill/enrichment + * query) share the same `$ids` variable name, so `"ids" in variables` alone can't disambiguate + * them either — the query string's selection set (`... on Issue { id body }`, unique to the + * dashboard-body query) is the only reliable signal, confirmed by inspecting the actual + * serialized request bodies at runtime. + */ +async function mockGraphqlWithDashboardBody( + page: import("@playwright/test").Page, + prs: Record[], + issues: Record[], + dashboardIssueId: string, + dashboardBody: string +) { + await page.route("https://api.github.com/graphql", (route) => { + const parsed = route.request().postDataJSON() as { query?: string } | null; + const query = parsed?.query ?? ""; + if (query.includes("on Issue { id body }")) { + return route.fulfill({ + status: 200, + json: { + data: { + nodes: [{ id: dashboardIssueId, body: dashboardBody }], + rateLimit: { limit: 5000, remaining: 4999, resetAt: "2099-01-01T00:00:00Z" }, + }, + }, + }); + } + return route.fulfill({ status: 200, json: graphqlWithDepPRs(prs, issues) }); + }); +} + // ── Dependencies tab visibility ───────────────────────────────────────────── test("dependencies tab auto-appears when dep bot PRs exist", async ({ page }) => { @@ -71,6 +134,83 @@ test("settings toggle hides the dependencies tab", async ({ page }) => { await expect(page.getByRole("tab", { name: /dependencies/i })).toHaveCount(0); }); +// ── Bot-detection signal isolation ────────────────────────────────────────── + +test("author-login-only detection: dependabot[bot] PR appears in Dependencies, not Pull Requests", async ({ page }) => { + const pr = makeDepPR({ + author: { login: "dependabot[bot]", avatarUrl: "https://avatars.githubusercontent.com/in/2740" }, + headRefName: "some-unrelated-branch", + title: "Fix something unrelated", + labels: { nodes: [] }, + }); + + await setupAuth(page); + await page.route("https://api.github.com/graphql", (route) => + route.fulfill({ status: 200, json: graphqlWithDepPRs([pr]) }) + ); + await page.goto("/dashboard"); + + await expect(page.getByRole("tab", { name: /dependencies/i })).toBeVisible({ timeout: 10_000 }); + await page.getByRole("tab", { name: /dependencies/i }).click(); + // Only the "mergeable" status group is expanded by default; these light, recent + // PRs land in "Needs Action" (collapsed), so expand all groups before asserting. + await page.getByLabel("Expand all repos").click(); + await expect(page.getByText(pr.title)).toBeVisible(); + + await page.getByRole("tab", { name: /pull requests/i }).click(); + await expect(page.getByText(pr.title)).toHaveCount(0); +}); + +test("branch-prefix-only detection: renovate/ branch PR appears in Dependencies, not Pull Requests", async ({ page }) => { + const pr = makeDepPR({ + author: { login: "some-human" }, + headRefName: "renovate/foo-1.x", + title: "Fix something unrelated", + labels: { nodes: [] }, + }); + + await setupAuth(page); + await page.route("https://api.github.com/graphql", (route) => + route.fulfill({ status: 200, json: graphqlWithDepPRs([pr]) }) + ); + await page.goto("/dashboard"); + + await expect(page.getByRole("tab", { name: /dependencies/i })).toBeVisible({ timeout: 10_000 }); + await page.getByRole("tab", { name: /dependencies/i }).click(); + // Only the "mergeable" status group is expanded by default; these light, recent + // PRs land in "Needs Action" (collapsed), so expand all groups before asserting. + await page.getByLabel("Expand all repos").click(); + await expect(page.getByText(pr.title)).toBeVisible(); + + await page.getByRole("tab", { name: /pull requests/i }).click(); + await expect(page.getByText(pr.title)).toHaveCount(0); +}); + +test("label-only detection: dependencies-labeled PR appears in Dependencies, not Pull Requests", async ({ page }) => { + const pr = makeDepPR({ + author: { login: "some-human" }, + headRefName: "some-feature", + title: "Fix something unrelated", + labels: { nodes: [{ name: "dependencies" }] }, + }); + + await setupAuth(page); + await page.route("https://api.github.com/graphql", (route) => + route.fulfill({ status: 200, json: graphqlWithDepPRs([pr]) }) + ); + await page.goto("/dashboard"); + + await expect(page.getByRole("tab", { name: /dependencies/i })).toBeVisible({ timeout: 10_000 }); + await page.getByRole("tab", { name: /dependencies/i }).click(); + // Only the "mergeable" status group is expanded by default; these light, recent + // PRs land in "Needs Action" (collapsed), so expand all groups before asserting. + await page.getByLabel("Expand all repos").click(); + await expect(page.getByText(pr.title)).toBeVisible(); + + await page.getByRole("tab", { name: /pull requests/i }).click(); + await expect(page.getByText(pr.title)).toHaveCount(0); +}); + // ── Status groups ─────────────────────────────────────────────────────────── test("status groups render correctly", async ({ page }) => { @@ -103,3 +243,45 @@ test("status groups render correctly", async ({ page }) => { await expect(page.getByText("Needs Action")).toBeVisible(); await expect(page.getByText("Stale")).toBeVisible(); }); + +// ── Abandoned-dependency pill ──────────────────────────────────────────────── + +test("abandoned dep badge links to the Dependency Dashboard issue when matched", async ({ page }) => { + const dashboardIssue = makeDashboardIssue(); + // makeDepPR()'s default title is "Bump lodash from 4.17.20 to 4.17.21" — matches + // the "lodash" row in ABANDONED_LODASH_BODY via matchAbandonedToPr()'s substring match. + const pr = makeDepPR(); + + await setupAuth(page); + await mockGraphqlWithDashboardBody(page, [pr], [dashboardIssue], dashboardIssue.id, ABANDONED_LODASH_BODY); + await page.goto("/dashboard"); + + await expect(page.getByRole("tab", { name: /dependencies/i })).toBeVisible({ timeout: 10_000 }); + await page.getByRole("tab", { name: /dependencies/i }).click(); + await page.getByLabel("Expand all repos").click(); + + // The dashboard-issue-body follow-up query only fires after the first full poll + // completes, so the badge starts as absent/plain-text and becomes a link asynchronously. + const abandonedLink = page.getByRole("link", { name: /abandoned/i }); + await expect(abandonedLink).toBeVisible({ timeout: 10_000 }); + await expect(abandonedLink).toHaveAttribute("href", dashboardIssue.url); +}); + +test("abandoned dep badge renders as plain text when title-suffix heuristic matches with no dashboard issue", async ({ page }) => { + const pr = makeDepPR({ + title: "Bump left-pad from 1.0.0 to 1.0.1 - abandoned", + }); + + await setupAuth(page); + await page.route("https://api.github.com/graphql", (route) => + route.fulfill({ status: 200, json: graphqlWithDepPRs([pr]) }) + ); + await page.goto("/dashboard"); + + await expect(page.getByRole("tab", { name: /dependencies/i })).toBeVisible({ timeout: 10_000 }); + await page.getByRole("tab", { name: /dependencies/i }).click(); + await page.getByLabel("Expand all repos").click(); + + await expect(page.getByText("Abandoned")).toBeVisible(); + await expect(page.getByRole("link", { name: /abandoned/i })).toHaveCount(0); +}); From 0fcf667bbd461f1010c2588cbe040f9d34efe604 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Mon, 27 Jul 2026 10:30:25 -0400 Subject: [PATCH 07/11] fix(ui): adds missing z-50 to IgnoreBadge popover content Kobalte portals Popover.Content to document.body, escaping the component's original stacking context. Without an explicit z-index the popover could render behind the fixed Header, sticky group headers, or fixed footer. Matches the z-50 convention already used by FilterPopover.tsx and the migrated SettingsPage Select triggers. --- src/app/components/dashboard/IgnoreBadge.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/components/dashboard/IgnoreBadge.tsx b/src/app/components/dashboard/IgnoreBadge.tsx index e4067eeb..ecf3acbd 100644 --- a/src/app/components/dashboard/IgnoreBadge.tsx +++ b/src/app/components/dashboard/IgnoreBadge.tsx @@ -55,7 +55,7 @@ export default function IgnoreBadge(props: IgnoreBadgeProps) {
From 316a2a176a2d7f98bfaf4b93bcf3f2445f45f404 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Mon, 27 Jul 2026 11:13:15 -0400 Subject: [PATCH 08/11] test(ui): guards z-50 stacking fix and per-item unignore popover behavior Adds regression coverage for two previously-untested IgnoreBadge behaviors found by quality-gate: the z-50 stacking-context fix, and the deliberate decision that individual Unignore clicks keep the popover open (only Unignore All closes it). --- tests/components/IgnoreBadge.test.tsx | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/tests/components/IgnoreBadge.test.tsx b/tests/components/IgnoreBadge.test.tsx index fe784243..1f8ac011 100644 --- a/tests/components/IgnoreBadge.test.tsx +++ b/tests/components/IgnoreBadge.test.tsx @@ -49,6 +49,11 @@ describe("IgnoreBadge", () => { // Now open expect(button.getAttribute("aria-expanded")).toBe("true"); + + // Regression guard: popover content must retain z-50 so it stacks above + // surrounding dashboard content (fix(ui): adds missing z-50 to IgnoreBadge popover content). + const content = document.querySelector("[aria-label='Ignored items']"); + expect(content?.className).toContain("z-50"); }); it("clicking badge again closes popover", async () => { @@ -84,14 +89,20 @@ describe("IgnoreBadge", () => { const onUnignore = vi.fn(); const items = [ makeIgnoredItem({ id: 123, title: "My Issue" }), + makeIgnoredItem({ id: 456, title: "Another Issue" }), ]; render(() => ); - await user.click(getTrigger(1)); + const button = getTrigger(2); + await user.click(button); - const unignoreBtn = screen.getByText("Unignore"); - await user.click(unignoreBtn); + const unignoreBtns = screen.getAllByText("Unignore"); + await user.click(unignoreBtns[0]); expect(onUnignore).toHaveBeenCalledWith(123); + // Deliberate behavior: individual "Unignore" clicks must NOT close the + // popover — only "Unignore All" does. Verify with 2+ items remaining so + // the click doesn't trivially empty the list. + expect(button.getAttribute("aria-expanded")).toBe("true"); }); it("'Unignore All' calls onUnignore for every item", async () => { From b9dc2e2565c942f9216eee5be91566a45d47bc74 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Mon, 27 Jul 2026 11:30:07 -0400 Subject: [PATCH 09/11] fix(settings): resets defaultTab when Dependencies tab is disabled Mirrors the existing reset guards for the tracked-items and GitHub-Actions toggles. Without this, disabling the Dependencies tab while it was the configured default tab left the migrated Kobalte Select trigger showing blank text (no option in the list matches the stale defaultTab value) until the user manually reselected a tab. Found by quality-gate adversarial review. --- src/app/components/settings/SettingsPage.tsx | 11 ++++++++++- .../components/settings/SettingsPage.test.tsx | 18 ++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/app/components/settings/SettingsPage.tsx b/src/app/components/settings/SettingsPage.tsx index 77e6dee7..8826ae88 100644 --- a/src/app/components/settings/SettingsPage.tsx +++ b/src/app/components/settings/SettingsPage.tsx @@ -1301,7 +1301,16 @@ export default function SettingsPage() { class="toggle toggle-primary" aria-label="Enable dependencies tab" checked={config.dependencies?.enabled ?? true} - onChange={() => saveWithFeedback({ dependencies: { ...config.dependencies, enabled: !(config.dependencies?.enabled ?? true) } })} + onChange={() => { + const val = !(config.dependencies?.enabled ?? true); + saveWithFeedback({ + dependencies: { ...config.dependencies, enabled: val }, + ...(!val && config.defaultTab === "dependencies" ? { defaultTab: "issues" as const } : {}), + }); + if (!val && viewState.lastActiveTab === "dependencies") { + updateViewState({ lastActiveTab: "issues" }); + } + }} /> { expect(config.dependencies.enabled).toBe(false); }); + it("disabling dependencies resets defaultTab to 'issues' when it was 'dependencies'", () => { + updateConfig({ dependencies: { enabled: true, rebaseLabel: "rebase" }, defaultTab: "dependencies" }); + renderSettings(); + const toggle = screen.getByRole("checkbox", { name: /Enable dependencies tab/i }); + fireEvent.click(toggle); + expect(config.dependencies.enabled).toBe(false); + expect(config.defaultTab).toBe("issues"); + }); + + it("disabling dependencies preserves defaultTab when it was not 'dependencies'", () => { + updateConfig({ dependencies: { enabled: true, rebaseLabel: "rebase" }, defaultTab: "pullRequests" }); + renderSettings(); + const toggle = screen.getByRole("checkbox", { name: /Enable dependencies tab/i }); + fireEvent.click(toggle); + expect(config.dependencies.enabled).toBe(false); + expect(config.defaultTab).toBe("pullRequests"); + }); + it("renders rebase label input with current value", () => { updateConfig({ dependencies: { enabled: true, rebaseLabel: "rebase-please" } }); renderSettings(); From 9d527d2e45cf93b93a11b77f0907f509ab317cc2 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Mon, 27 Jul 2026 11:34:43 -0400 Subject: [PATCH 10/11] test(settings): guards lastActiveTab reset on Dependencies toggle disable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes coverage for the b9dc2e2 fix — the code path resetting viewState.lastActiveTab was correct but untested. Found by Layer 2 completeness re-review (Pass 2). --- tests/components/settings/SettingsPage.test.tsx | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/components/settings/SettingsPage.test.tsx b/tests/components/settings/SettingsPage.test.tsx index 7379e1c9..ac5240da 100644 --- a/tests/components/settings/SettingsPage.test.tsx +++ b/tests/components/settings/SettingsPage.test.tsx @@ -1206,6 +1206,16 @@ describe("Dependencies settings section", () => { expect(config.defaultTab).toBe("pullRequests"); }); + it("disabling dependencies resets lastActiveTab to 'issues' when it was 'dependencies'", () => { + updateConfig({ dependencies: { enabled: true, rebaseLabel: "rebase" } }); + updateViewState({ lastActiveTab: "dependencies" }); + renderSettings(); + const toggle = screen.getByRole("checkbox", { name: /Enable dependencies tab/i }); + fireEvent.click(toggle); + expect(config.dependencies.enabled).toBe(false); + expect(viewState.lastActiveTab).toBe("issues"); + }); + it("renders rebase label input with current value", () => { updateConfig({ dependencies: { enabled: true, rebaseLabel: "rebase-please" } }); renderSettings(); From a26dd58307abe050b87e6701496d4e9456929bb4 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Mon, 27 Jul 2026 13:48:40 -0400 Subject: [PATCH 11/11] fix(jira): resets stale custom scope reference on disconnect or deletion Closes BUG-008. viewState.tabFilters.jiraAssigned.scope could hold a reference to a custom Jira scope ID that no longer exists after (1) disconnecting Jira (clearJiraConfigFull wipes customScopes but left the tab filter untouched), or (2) deleting the active custom scope via JiraScopePicker. Either path let a stale scope ID survive into the next fetchJiraAssigned() call, which fired an invalid JQL query and surfaced a confusing 'Custom scope not supported' toast before the existing error-handler/tab-mount guards self-healed it. Extracts the validity check into a pure, exported staleJiraScopeReset() helper shared by both call sites (handleJiraDisconnect and the scope picker's onSave), matching the same built-in-scope list already used by JiraAssignedTab's reactive guard. Adds direct unit tests for the helper plus two integration tests covering the disconnect path. --- src/app/components/settings/SettingsPage.tsx | 33 +++++++++++- .../components/settings/JiraSection.test.tsx | 52 ++++++++++++++++++- 2 files changed, 81 insertions(+), 4 deletions(-) diff --git a/src/app/components/settings/SettingsPage.tsx b/src/app/components/settings/SettingsPage.tsx index 8826ae88..0cad0b09 100644 --- a/src/app/components/settings/SettingsPage.tsx +++ b/src/app/components/settings/SettingsPage.tsx @@ -4,7 +4,8 @@ import * as Sentry from "@sentry/solid"; import { getRelayStatus } from "../../lib/mcp-relay"; import { useNavigate } from "@solidjs/router"; import { config, updateConfig, updateJiraConfig, updateJiraCustomFields, updateJiraCustomScopes, setMonitoredRepo, isActionsBasedTab } from "../../stores/config"; -import { viewState, updateViewState } from "../../stores/view"; +import type { JiraCustomField } from "../../../shared/schemas"; +import { viewState, updateViewState, setTabFilter } from "../../stores/view"; import { clearAuth, jiraAuth, setJiraAuth, clearJiraConfigFull, isJiraAuthenticated, token, setAuthFromPat } from "../../stores/auth"; import type { GitHubUser } from "../../stores/auth"; import { isValidPatFormat } from "../../lib/pat"; @@ -60,6 +61,21 @@ export const SETTINGS_PAGE_SECTION_IDS = [ "data", ] as const; +// Built-in Jira scope values, duplicated from JiraAssignedTab.tsx's BUILTIN_SCOPE_OPTIONS +// (not exported there to avoid a cross-component coupling for 3 literal strings). +const BUILTIN_JIRA_SCOPES = ["assigned", "reported", "watching"]; + +/** + * Returns the scope value to fall back to if `activeScope` no longer exists in + * `scopes` (e.g. the custom scope backing it was just deleted, or Jira was + * disconnected and `scopes` is empty) — otherwise null. Guards against firing + * an invalid JQL query with a stale custom scope ID (BUG-008). + */ +export function staleJiraScopeReset(activeScope: string, scopes: JiraCustomField[]): string | null { + const validScopeIds = new Set([...BUILTIN_JIRA_SCOPES, ...scopes.map((s) => s.id)]); + return validScopeIds.has(activeScope) ? null : "assigned"; +} + export default function SettingsPage() { const navigate = useNavigate(); @@ -433,6 +449,11 @@ export default function SettingsPage() { if (viewState.lastActiveTab === "jiraAssigned") { updateViewState({ lastActiveTab: "issues" }); } + // Stale scope guard: clearJiraConfigFull() wipes customScopes, so a tab + // filter pointing at a custom scope ID would otherwise survive reconnect + // and fire an invalid JQL query against the new Jira instance (BUG-008). + const reset = staleJiraScopeReset(viewState.tabFilters.jiraAssigned.scope, []); + if (reset) setTabFilter("jiraAssigned", "scope", reset); } // ── Refresh interval options ────────────────────────────────────────────── @@ -1270,7 +1291,15 @@ export default function SettingsPage() { { updateJiraCustomScopes(scopes); setShowScopePicker(false); }} + onSave={(scopes) => { + updateJiraCustomScopes(scopes); + setShowScopePicker(false); + // Stale scope guard: if the active Jira tab filter pointed at a + // custom scope that was just removed, reset it before it can fire + // an invalid JQL query (BUG-008). + const reset = staleJiraScopeReset(viewState.tabFilters.jiraAssigned.scope, scopes); + if (reset) setTabFilter("jiraAssigned", "scope", reset); + }} onCancel={() => setShowScopePicker(false)} />
diff --git a/tests/components/settings/JiraSection.test.tsx b/tests/components/settings/JiraSection.test.tsx index 081bf22e..a014f7b7 100644 --- a/tests/components/settings/JiraSection.test.tsx +++ b/tests/components/settings/JiraSection.test.tsx @@ -103,9 +103,10 @@ vi.mock("../../../src/app/stores/config", () => ({ })); vi.mock("../../../src/app/stores/view", () => ({ - viewState: { lastActiveTab: "issues", tabFilters: {}, expandedRepos: {}, lockedRepos: {}, trackedItems: [], activeScopeTab: "involved" }, + viewState: { lastActiveTab: "issues", tabFilters: { jiraAssigned: { scope: "assigned" } }, expandedRepos: {}, lockedRepos: {}, trackedItems: [], activeScopeTab: "involved" }, updateViewState: vi.fn(), resetViewState: vi.fn(), + setTabFilter: vi.fn(), ViewStateSchema: { parse: vi.fn((x: unknown) => x) }, })); @@ -163,7 +164,8 @@ vi.mock("../../../src/app/services/jira-keys", () => ({ })); // Component imports after all mocks -import SettingsPage from "../../../src/app/components/settings/SettingsPage"; +import SettingsPage, { staleJiraScopeReset } from "../../../src/app/components/settings/SettingsPage"; +import { viewState, setTabFilter } from "../../../src/app/stores/view"; // ── Helpers ─────────────────────────────────────────────────────────────────── @@ -525,6 +527,31 @@ describe("SettingsPage Jira section — connected state", () => { expect(mockClearJiraConfigFull).toHaveBeenCalled(); }); + it("Disconnect resets a stale custom jiraAssigned scope to 'assigned' (BUG-008)", async () => { + viewState.tabFilters.jiraAssigned.scope = "customfield_10001"; + renderSettings(); + await waitFor(() => { + expect(screen.getByRole("button", { name: /Disconnect/i })).toBeTruthy(); + }); + + fireEvent.click(screen.getByRole("button", { name: /Disconnect/i })); + + expect(setTabFilter).toHaveBeenCalledWith("jiraAssigned", "scope", "assigned"); + }); + + it("Disconnect does not touch jiraAssigned scope when it's already 'assigned'", async () => { + viewState.tabFilters.jiraAssigned.scope = "assigned"; + renderSettings(); + await waitFor(() => { + expect(screen.getByRole("button", { name: /Disconnect/i })).toBeTruthy(); + }); + + vi.mocked(setTabFilter).mockClear(); + fireEvent.click(screen.getByRole("button", { name: /Disconnect/i })); + + expect(setTabFilter).not.toHaveBeenCalled(); + }); + it("Disconnect does not show OAuth connect buttons (only when disconnected)", async () => { renderSettings(); await waitFor(() => { @@ -535,3 +562,24 @@ describe("SettingsPage Jira section — connected state", () => { expect(screen.queryByText(/Use API token/i)).toBeNull(); }); }); + +describe("staleJiraScopeReset (BUG-008)", () => { + it("returns null when the active scope is a built-in value", () => { + expect(staleJiraScopeReset("assigned", [])).toBeNull(); + expect(staleJiraScopeReset("reported", [{ id: "customfield_1", name: "Foo" }])).toBeNull(); + expect(staleJiraScopeReset("watching", [])).toBeNull(); + }); + + it("returns null when the active scope matches a surviving custom scope id", () => { + expect( + staleJiraScopeReset("customfield_10001", [{ id: "customfield_10001", name: "Epic Filter" }]) + ).toBeNull(); + }); + + it("returns 'assigned' when the active scope is a custom id no longer present", () => { + expect(staleJiraScopeReset("customfield_10001", [])).toBe("assigned"); + expect( + staleJiraScopeReset("customfield_10001", [{ id: "customfield_9999", name: "Other" }]) + ).toBe("assigned"); + }); +});