diff --git a/src/app/components/dashboard/DashboardPage.tsx b/src/app/components/dashboard/DashboardPage.tsx index bfc2408a..393ec83c 100644 --- a/src/app/components/dashboard/DashboardPage.tsx +++ b/src/app/components/dashboard/DashboardPage.tsx @@ -19,7 +19,8 @@ import { fetchAllData, type DashboardData, } from "../../services/poll"; -import { clearAuth, user, onAuthCleared, DASHBOARD_STORAGE_KEY } from "../../stores/auth"; +import { expireToken, user, onAuthCleared, DASHBOARD_STORAGE_KEY } from "../../stores/auth"; +import { pushNotification } from "../../lib/errors"; import { getClient, getGraphqlRateLimit } from "../../services/github"; import { formatCount } from "../../lib/format"; import { setsEqual } from "../../lib/collections"; @@ -191,7 +192,7 @@ async function pollFetch(): Promise { try { localStorage.setItem(DASHBOARD_STORAGE_KEY, JSON.stringify(cachePayload)); } catch { - // localStorage full or unavailable — non-fatal + pushNotification("localStorage:dashboard", "Dashboard cache write failed — storage may be full", "warning"); } }, 0); } else { @@ -208,9 +209,9 @@ async function pollFetch(): Promise { : null; if (status === 401) { - // Hard redirect (not navigate()) forces a full page reload, which clears - // module-level state like _coordinator and dashboardData for the next user. - clearAuth(); + // Token invalid — clear token only, preserve user config/view/dashboard. + // Hard redirect forces a full page reload, clearing module-level state. + expireToken(); window.location.replace("/login"); } setDashboardData("loading", false); @@ -359,6 +360,7 @@ export default function DashboardPage() { userLogin={userLogin()} allUsers={allUsers()} trackedUsers={config.trackedUsers} + monitoredRepos={config.monitoredRepos} /> @@ -369,6 +371,7 @@ export default function DashboardPage() { allUsers={allUsers()} trackedUsers={config.trackedUsers} hotPollingPRIds={hotPollingPRIds()} + monitoredRepos={config.monitoredRepos} /> diff --git a/src/app/components/dashboard/IssuesTab.tsx b/src/app/components/dashboard/IssuesTab.tsx index 77491b87..2137958e 100644 --- a/src/app/components/dashboard/IssuesTab.tsx +++ b/src/app/components/dashboard/IssuesTab.tsx @@ -1,7 +1,7 @@ import { createEffect, createMemo, createSignal, For, Show } from "solid-js"; import { config, type TrackedUser } from "../../stores/config"; import { viewState, setSortPreference, setTabFilter, resetTabFilter, resetAllTabFilters, ignoreItem, unignoreItem, toggleExpandedRepo, setAllExpanded, pruneExpandedRepos, pruneLockedRepos, type IssueFilterField } from "../../stores/view"; -import type { Issue } from "../../services/api"; +import type { Issue, RepoRef } from "../../services/api"; import ItemRow from "./ItemRow"; import UserAvatarBadge, { buildSurfacedByUsers } from "../shared/UserAvatarBadge"; import IgnoreBadge from "./IgnoreBadge"; @@ -25,6 +25,7 @@ export interface IssuesTabProps { userLogin: string; allUsers?: { login: string; label: string }[]; trackedUsers?: TrackedUser[]; + monitoredRepos?: RepoRef[]; } type SortField = "repo" | "title" | "author" | "createdAt" | "updatedAt" | "comments"; @@ -68,6 +69,10 @@ export default function IssuesTab(props: IssuesTabProps) { new Set((config.upstreamRepos ?? []).map(r => r.fullName)) ); + const monitoredRepoNameSet = createMemo(() => + new Set((props.monitoredRepos ?? []).map(r => r.fullName)) + ); + const filterGroups = createMemo(() => { const users = props.allUsers; if (!users || users.length <= 1) return issueFilterGroups; @@ -114,10 +119,13 @@ export default function IssuesTab(props: IssuesTabProps) { } if (tabFilter.user !== "all") { - const validUser = !props.allUsers || props.allUsers.some(u => u.login === tabFilter.user); - if (validUser) { - const surfacedBy = issue.surfacedBy ?? [props.userLogin.toLowerCase()]; - if (!surfacedBy.includes(tabFilter.user)) return false; + // Items from monitored repos bypass the surfacedBy filter (all activity is shown) + if (!monitoredRepoNameSet().has(issue.repoFullName)) { + const validUser = !props.allUsers || props.allUsers.some(u => u.login === tabFilter.user); + if (validUser) { + const surfacedBy = issue.surfacedBy ?? [props.userLogin.toLowerCase()]; + if (!surfacedBy.includes(tabFilter.user)) return false; + } } } @@ -305,6 +313,9 @@ export default function IssuesTab(props: IssuesTabProps) { > {repoGroup.repoFullName} + + Monitoring all + {repoGroup.items.length} {repoGroup.items.length === 1 ? "issue" : "issues"} diff --git a/src/app/components/dashboard/PullRequestsTab.tsx b/src/app/components/dashboard/PullRequestsTab.tsx index 8d8e36b0..a52c484a 100644 --- a/src/app/components/dashboard/PullRequestsTab.tsx +++ b/src/app/components/dashboard/PullRequestsTab.tsx @@ -1,7 +1,7 @@ import { createEffect, createMemo, createSignal, For, Show } from "solid-js"; import { config, type TrackedUser } from "../../stores/config"; import { viewState, setSortPreference, ignoreItem, unignoreItem, setTabFilter, resetTabFilter, resetAllTabFilters, toggleExpandedRepo, setAllExpanded, pruneExpandedRepos, pruneLockedRepos, type PullRequestFilterField } from "../../stores/view"; -import type { PullRequest } from "../../services/api"; +import type { PullRequest, RepoRef } from "../../services/api"; import { deriveInvolvementRoles, prSizeCategory } from "../../lib/format"; import ExpandCollapseButtons from "../shared/ExpandCollapseButtons"; import ItemRow from "./ItemRow"; @@ -30,6 +30,7 @@ export interface PullRequestsTabProps { allUsers?: { login: string; label: string }[]; trackedUsers?: TrackedUser[]; hotPollingPRIds?: ReadonlySet; + monitoredRepos?: RepoRef[]; } type SortField = "repo" | "title" | "author" | "createdAt" | "updatedAt" | "checkStatus" | "reviewDecision" | "size"; @@ -135,6 +136,10 @@ export default function PullRequestsTab(props: PullRequestsTabProps) { new Set((config.upstreamRepos ?? []).map(r => r.fullName)) ); + const monitoredRepoNameSet = createMemo(() => + new Set((props.monitoredRepos ?? []).map(r => r.fullName)) + ); + const filterGroups = createMemo(() => { const users = props.allUsers; if (!users || users.length <= 1) return prFilterGroups; @@ -199,10 +204,13 @@ export default function PullRequestsTab(props: PullRequestsTabProps) { } if (tabFilters.user !== "all") { - const validUser = !props.allUsers || props.allUsers.some(u => u.login === tabFilters.user); - if (validUser) { - const surfacedBy = pr.surfacedBy ?? [props.userLogin.toLowerCase()]; - if (!surfacedBy.includes(tabFilters.user)) return false; + // Items from monitored repos bypass the surfacedBy filter (all activity is shown) + if (!monitoredRepoNameSet().has(pr.repoFullName)) { + const validUser = !props.allUsers || props.allUsers.some(u => u.login === tabFilters.user); + if (validUser) { + const surfacedBy = pr.surfacedBy ?? [props.userLogin.toLowerCase()]; + if (!surfacedBy.includes(tabFilters.user)) return false; + } } } @@ -418,6 +426,9 @@ export default function PullRequestsTab(props: PullRequestsTabProps) { > {repoGroup.repoFullName} + + Monitoring all + {repoGroup.items.length} {repoGroup.items.length === 1 ? "PR" : "PRs"} diff --git a/src/app/components/onboarding/OnboardingWizard.tsx b/src/app/components/onboarding/OnboardingWizard.tsx index 68edc3df..480f9dc2 100644 --- a/src/app/components/onboarding/OnboardingWizard.tsx +++ b/src/app/components/onboarding/OnboardingWizard.tsx @@ -7,6 +7,7 @@ import { Match, } from "solid-js"; import { config, updateConfig, CONFIG_STORAGE_KEY } from "../../stores/config"; +import { pushNotification } from "../../lib/errors"; import { fetchOrgs, type OrgEntry, type RepoRef } from "../../services/api"; import { getClient } from "../../services/github"; import RepoSelector from "./RepoSelector"; @@ -19,6 +20,9 @@ export default function OnboardingWizard() { const [upstreamRepos, setUpstreamRepos] = createSignal( config.upstreamRepos.length > 0 ? [...config.upstreamRepos] : [] ); + const [monitoredRepos, setMonitoredRepos] = createSignal( + config.monitoredRepos.length > 0 ? [...config.monitoredRepos] : [] + ); const [loading, setLoading] = createSignal(true); const [error, setError] = createSignal(null); @@ -57,10 +61,15 @@ export default function OnboardingWizard() { selectedOrgs: uniqueOrgs, selectedRepos: selectedRepos(), upstreamRepos: upstreamRepos(), + monitoredRepos: monitoredRepos(), onboardingComplete: true, }); // Flush synchronously — the debounced persistence effect won't fire before page unload - localStorage.setItem(CONFIG_STORAGE_KEY, JSON.stringify(config)); + try { + localStorage.setItem(CONFIG_STORAGE_KEY, JSON.stringify(config)); + } catch { + pushNotification("localStorage:config", "Config write failed — storage may be full", "warning"); + } window.location.replace("/dashboard"); } @@ -116,6 +125,14 @@ export default function OnboardingWizard() { upstreamRepos={upstreamRepos()} onUpstreamChange={setUpstreamRepos} trackedUsers={config.trackedUsers} + monitoredRepos={monitoredRepos()} + onMonitorToggle={(repo, monitored) => { + if (monitored) { + setMonitoredRepos((prev) => prev.some((r) => r.fullName === repo.fullName) ? prev : [...prev, repo]); + } else { + setMonitoredRepos((prev) => prev.filter((r) => r.fullName !== repo.fullName)); + } + }} /> diff --git a/src/app/components/onboarding/RepoSelector.tsx b/src/app/components/onboarding/RepoSelector.tsx index d5227c61..ab0e26de 100644 --- a/src/app/components/onboarding/RepoSelector.tsx +++ b/src/app/components/onboarding/RepoSelector.tsx @@ -10,6 +10,7 @@ import { import { fetchOrgs, fetchRepos, discoverUpstreamRepos, OrgEntry, RepoRef, RepoEntry } from "../../services/api"; import { getClient } from "../../services/github"; import { user } from "../../stores/auth"; +import type { TrackedUser } from "../../stores/config"; import { relativeTime } from "../../lib/format"; import LoadingSpinner from "../shared/LoadingSpinner"; import FilterInput from "../shared/FilterInput"; @@ -25,7 +26,9 @@ interface RepoSelectorProps { showUpstreamDiscovery?: boolean; upstreamRepos?: RepoRef[]; onUpstreamChange?: (repos: RepoRef[]) => void; - trackedUsers?: { login: string; avatarUrl: string; name: string | null }[]; + trackedUsers?: TrackedUser[]; + monitoredRepos?: RepoRef[]; + onMonitorToggle?: (repo: RepoRef, monitored: boolean) => void; } interface OrgRepoState { @@ -225,6 +228,10 @@ export default function RepoSelector(props: RepoSelectorProps) { new Set(props.selected.map((r) => r.fullName)) ); + const monitoredSet = createMemo(() => + new Set((props.monitoredRepos ?? []).map((r) => r.fullName)) + ); + const sortedOrgStates = createMemo(() => { const states = orgStates(); // Defer sorting until all orgs have loaded: prevents layout shift during @@ -527,26 +534,51 @@ export default function RepoSelector(props: RepoSelectorProps) { {(repo) => { return (
  • -
    • ); }} diff --git a/src/app/components/settings/SettingsPage.tsx b/src/app/components/settings/SettingsPage.tsx index 1eba20d9..f3db7e9c 100644 --- a/src/app/components/settings/SettingsPage.tsx +++ b/src/app/components/settings/SettingsPage.tsx @@ -1,6 +1,6 @@ -import { createSignal, Show, onCleanup } from "solid-js"; +import { createSignal, createMemo, Show, onCleanup } from "solid-js"; import { useNavigate } from "@solidjs/router"; -import { config, updateConfig } from "../../stores/config"; +import { config, updateConfig, setMonitoredRepo } from "../../stores/config"; import { clearAuth } from "../../stores/auth"; import { clearCache } from "../../stores/cache"; import { pushNotification } from "../../lib/errors"; @@ -54,6 +54,10 @@ export default function SettingsPage() { const [localRepos, setLocalRepos] = createSignal(config.selectedRepos); const [localUpstream, setLocalUpstream] = createSignal(config.upstreamRepos); + const monitoredRepoNames = createMemo(() => + config.monitoredRepos.map(r => r.fullName).join(", ") + ); + // ── Helpers ────────────────────────────────────────────────────────────── async function mergeNewOrgs() { @@ -142,6 +146,7 @@ export default function SettingsPage() { selectedOrgs: config.selectedOrgs, selectedRepos: config.selectedRepos, upstreamRepos: config.upstreamRepos, + monitoredRepos: config.monitoredRepos, trackedUsers: config.trackedUsers, refreshInterval: config.refreshInterval, hotPollInterval: config.hotPollInterval, @@ -304,6 +309,15 @@ export default function SettingsPage() { Tracking {localRepos().length} repos will use significant API quota per poll cycle

    + 0}> +

    + + Monitoring all: {monitoredRepoNames()} +

    +     + Repos: {props.selected.length} + Monitored: {(props.monitoredRepos ?? []).length} ), })); @@ -47,7 +60,7 @@ vi.mock("../../../src/app/components/shared/LoadingSpinner", () => ({ // Mock config store vi.mock("../../../src/app/stores/config", () => ({ CONFIG_STORAGE_KEY: "github-tracker:config", - config: { selectedOrgs: [], selectedRepos: [], upstreamRepos: [] }, + config: { selectedOrgs: [], selectedRepos: [], upstreamRepos: [], monitoredRepos: [], trackedUsers: [] }, updateConfig: vi.fn(), })); @@ -258,4 +271,27 @@ describe("OnboardingWizard", () => { await user.click(screen.getByText(/Finish Setup \(1 repo\)/)); expect(window.location.replace).toHaveBeenCalledWith("/dashboard"); }); + + it("passes monitoredRepos to updateConfig on finish (C4)", async () => { + const user = userEvent.setup(); + vi.mocked(apiModule.fetchOrgs).mockResolvedValue(mockOrgs); + render(() => ); + + await waitFor(() => screen.getByTestId("repo-selector")); + + // Select a repo first + await user.click(screen.getByText("Select Repo")); + // Toggle monitor for that repo + await user.click(screen.getByText("Toggle Monitor")); + + await waitFor(() => screen.getByText(/Finish Setup \(1 repo\)/)); + await user.click(screen.getByText(/Finish Setup \(1 repo\)/)); + + expect(configStore.updateConfig).toHaveBeenCalledWith( + expect.objectContaining({ + monitoredRepos: [{ owner: "myorg", name: "myrepo", fullName: "myorg/myrepo" }], + selectedRepos: [{ owner: "myorg", name: "myrepo", fullName: "myorg/myrepo" }], + }) + ); + }); }); diff --git a/tests/components/onboarding/RepoSelector.test.tsx b/tests/components/onboarding/RepoSelector.test.tsx index 9544e1fb..1ccf2154 100644 --- a/tests/components/onboarding/RepoSelector.test.tsx +++ b/tests/components/onboarding/RepoSelector.test.tsx @@ -691,3 +691,111 @@ describe("RepoSelector — upstream discovery", () => { }); }); }); + +// ── Monitor toggle (C4) ──────────────────────────────────────────────────────── + +describe("RepoSelector — monitor toggle", () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(api.fetchRepos).mockImplementation((_client, org) => { + if (org === "myorg") return Promise.resolve(myorgRepos); + return Promise.resolve([]); + }); + }); + + it("does not render monitor toggle when onMonitorToggle prop is absent", async () => { + const selected: RepoRef[] = [{ owner: "myorg", name: "repo-a", fullName: "myorg/repo-a" }]; + render(() => ( + + )); + + await waitFor(() => screen.getByText("repo-a")); + expect(screen.queryByLabelText(/monitor all activity/i)).toBeNull(); + }); + + it("renders monitor toggle only for selected repos", async () => { + const selected: RepoRef[] = [{ owner: "myorg", name: "repo-a", fullName: "myorg/repo-a" }]; + // repo-b is not selected + render(() => ( + + )); + + await waitFor(() => screen.getByText("repo-a")); + // Toggle for selected repo-a should be present + screen.getByLabelText("Monitor all activity"); + // Toggle for unselected repo-b should not be present + expect(screen.queryAllByLabelText(/monitor all activity/i)).toHaveLength(1); + }); + + it("calls onMonitorToggle with repo and monitored=true when repo is not monitored", async () => { + const onMonitorToggle = vi.fn(); + const selected: RepoRef[] = [{ owner: "myorg", name: "repo-a", fullName: "myorg/repo-a" }]; + render(() => ( + + )); + + await waitFor(() => screen.getByText("repo-a")); + const btn = screen.getByLabelText("Monitor all activity"); + btn.click(); + expect(onMonitorToggle).toHaveBeenCalledWith( + { owner: "myorg", name: "repo-a", fullName: "myorg/repo-a" }, + true + ); + }); + + it("calls onMonitorToggle with monitored=false when repo is already monitored", async () => { + const onMonitorToggle = vi.fn(); + const selected: RepoRef[] = [{ owner: "myorg", name: "repo-a", fullName: "myorg/repo-a" }]; + const monitoredRepos: RepoRef[] = [{ owner: "myorg", name: "repo-a", fullName: "myorg/repo-a" }]; + render(() => ( + + )); + + await waitFor(() => screen.getByText("repo-a")); + const btn = screen.getByLabelText("Stop monitoring all activity"); + btn.click(); + expect(onMonitorToggle).toHaveBeenCalledWith( + { owner: "myorg", name: "repo-a", fullName: "myorg/repo-a" }, + false + ); + }); + + it("hides monitor toggle for upstream repos even when selected", async () => { + const selected: RepoRef[] = [{ owner: "myorg", name: "repo-a", fullName: "myorg/repo-a" }]; + const upstreamRepos: RepoRef[] = [{ owner: "myorg", name: "repo-a", fullName: "myorg/repo-a" }]; + render(() => ( + + )); + + await waitFor(() => screen.getByText("repo-a")); + // Monitor toggle should not appear for upstream repos + expect(screen.queryByLabelText(/monitor all activity/i)).toBeNull(); + }); +}); diff --git a/tests/components/settings/SettingsPage.test.tsx b/tests/components/settings/SettingsPage.test.tsx index 8d96688a..1e8ae380 100644 --- a/tests/components/settings/SettingsPage.test.tsx +++ b/tests/components/settings/SettingsPage.test.tsx @@ -682,3 +682,56 @@ describe("SettingsPage — Manage org access button", () => { expect(apiModule.fetchOrgs).toHaveBeenCalledTimes(1); }); }); + +describe("SettingsPage — monitor toggle wiring", () => { + it("shows monitored repos indicator when repos are monitored", () => { + updateConfig({ + selectedRepos: [ + { owner: "org", name: "repo1", fullName: "org/repo1" }, + { owner: "org", name: "repo2", fullName: "org/repo2" }, + ], + monitoredRepos: [ + { owner: "org", name: "repo1", fullName: "org/repo1" }, + { owner: "org", name: "repo2", fullName: "org/repo2" }, + ], + }); + renderSettings(); + + const indicator = screen.getByText(/Monitoring all:/); + expect(indicator.textContent).toContain("org/repo1"); + expect(indicator.textContent).toContain("org/repo2"); + }); + + it("hides monitored repos indicator when no repos are monitored", () => { + updateConfig({ monitoredRepos: [] }); + renderSettings(); + + expect(screen.queryByText(/Monitoring all:/)).toBeNull(); + }); + + it("includes monitoredRepos in exported settings JSON", async () => { + updateConfig({ + selectedRepos: [{ owner: "org", name: "repo1", fullName: "org/repo1" }], + monitoredRepos: [{ owner: "org", name: "repo1", fullName: "org/repo1" }], + }); + renderSettings(); + + // Trigger export + const exportBtn = screen.getByRole("button", { name: /export/i }); + const user = userEvent.setup(); + const blobParts: BlobPart[] = []; + const originalBlob = globalThis.Blob; + globalThis.Blob = class MockBlob extends originalBlob { + constructor(parts?: BlobPart[], options?: BlobPropertyBag) { + super(parts, options); + if (parts) blobParts.push(...parts); + } + } as typeof Blob; + + await user.click(exportBtn); + + globalThis.Blob = originalBlob; + const json = JSON.parse(blobParts[0] as string); + expect(json.monitoredRepos).toEqual([{ owner: "org", name: "repo1", fullName: "org/repo1" }]); + }); +}); diff --git a/tests/components/settings/TrackedUsersSection.test.tsx b/tests/components/settings/TrackedUsersSection.test.tsx index 1b7549ca..500c0fd7 100644 --- a/tests/components/settings/TrackedUsersSection.test.tsx +++ b/tests/components/settings/TrackedUsersSection.test.tsx @@ -50,6 +50,7 @@ function makeUser(overrides: Partial = {}): TrackedUser { login: "octocat", avatarUrl: "https://avatars.githubusercontent.com/u/583231", name: "The Octocat", + type: "user", ...overrides, }; } @@ -306,3 +307,59 @@ describe("TrackedUsersSection — removing a user", () => { screen.getByRole("button", { name: /remove user3/i }); }); }); + +// ── Bot badge UI (C2) ───────────────────────────────────────────────────────── + +describe("TrackedUsersSection — bot badge", () => { + it("renders bot badge for type:bot user", () => { + const users = [ + makeUser({ login: "dependabot[bot]", name: null, type: "bot" }), + ]; + render(() => ); + screen.getByLabelText("dependabot[bot] is a bot account"); + }); + + it("does not render bot badge for type:user", () => { + const users = [makeUser({ login: "octocat", type: "user" })]; + render(() => ); + expect(screen.queryByText("bot")).toBeNull(); + }); + + it("renders bot badge only for bot users in mixed list", () => { + const users = [ + makeUser({ login: "octocat", type: "user" }), + makeUser({ login: "dependabot[bot]", name: null, type: "bot", avatarUrl: "https://avatars.githubusercontent.com/u/27347476" }), + ]; + render(() => ); + const badges = screen.getAllByLabelText("dependabot[bot] is a bot account"); + expect(badges).toHaveLength(1); + }); + + it("renders bot badge text 'bot'", () => { + const users = [ + makeUser({ login: "khepri-bot[bot]", name: null, type: "bot" }), + ]; + render(() => ); + const badge = screen.getByLabelText("khepri-bot[bot] is a bot account"); + expect(badge.textContent).toBe("bot"); + }); +}); + +describe("TrackedUsersSection — bot input handling", () => { + it("lowercases bot login with [bot] suffix before calling validateGitHubUser", async () => { + const botUser = makeUser({ login: "khepri-bot[bot]", name: null, type: "bot" }); + vi.mocked(apiModule.validateGitHubUser).mockResolvedValue(botUser); + + const onSave = vi.fn(); + render(() => ); + + const input = screen.getByRole("textbox", { name: /github username/i }) as HTMLInputElement; + // Use fireEvent.input instead of userEvent.type — userEvent interprets [bot] as a special key sequence + fireEvent.input(input, { target: { value: "Khepri-Bot[bot]" } }); + fireEvent.click(screen.getByRole("button", { name: /add/i })); + + await waitFor(() => { + expect(apiModule.validateGitHubUser).toHaveBeenCalledWith(expect.anything(), "khepri-bot[bot]"); + }); + }); +}); diff --git a/tests/helpers/index.tsx b/tests/helpers/index.tsx index 66430c3c..144853fc 100644 --- a/tests/helpers/index.tsx +++ b/tests/helpers/index.tsx @@ -53,6 +53,7 @@ export function makePullRequest(overrides: Partial = {}): PullReque labels: [], reviewDecision: null, totalReviewCount: 0, + enriched: true, surfacedBy: ["testuser"], ...overrides, }; diff --git a/tests/services/api-users.test.ts b/tests/services/api-users.test.ts index 70e0408c..8ff0a8f3 100644 --- a/tests/services/api-users.test.ts +++ b/tests/services/api-users.test.ts @@ -38,12 +38,14 @@ function makeUserResponse(overrides: { login?: string; avatar_url?: string; name?: string | null; + type?: string; } = {}) { return { data: { login: overrides.login ?? "octocat", avatar_url: overrides.avatar_url ?? "https://avatars.githubusercontent.com/u/583231?v=4", name: overrides.name !== undefined ? overrides.name : "The Octocat", + type: overrides.type ?? "User", }, }; } @@ -105,6 +107,7 @@ describe("validateGitHubUser", () => { login: "octocat", avatarUrl: "https://avatars.githubusercontent.com/u/583231?v=4", name: "The Octocat", + type: "user", }); expect(octokit.request).toHaveBeenCalledWith("GET /users/{username}", { username: "octocat" }); }); @@ -489,6 +492,7 @@ function makeTrackedUser(login: string): TrackedUser { login, avatarUrl: `https://avatars.githubusercontent.com/u/99999`, name: login, + type: "user", }; } diff --git a/tests/services/api.test.ts b/tests/services/api.test.ts index 13affff2..0eb6de64 100644 --- a/tests/services/api.test.ts +++ b/tests/services/api.test.ts @@ -6,6 +6,7 @@ import { fetchIssues, fetchPullRequests, fetchWorkflowRuns, + validateGitHubUser, type RepoRef, } from "../../src/app/services/api"; import { clearCache } from "../../src/app/stores/cache"; @@ -112,7 +113,7 @@ describe("fetchRepos", () => { it("returns repos for an org via paginate.iterator", async () => { const octokit = makeBasicOctokit(); const result = await fetchRepos( - octokit as unknown as ReturnType, + octokit as never, "acme-corp", "org" ); @@ -129,7 +130,7 @@ describe("fetchRepos", () => { it("passes sort=pushed and direction=desc to paginate.iterator", async () => { const octokit = makeBasicOctokit(); await fetchRepos( - octokit as unknown as ReturnType, + octokit as never, "acme-corp", "org" ); @@ -142,7 +143,7 @@ describe("fetchRepos", () => { it("passes sort=pushed and direction=desc for user repos", async () => { const octokit = makeBasicOctokit(); await fetchRepos( - octokit as unknown as ReturnType, + octokit as never, "octocat", "user" ); @@ -155,7 +156,7 @@ describe("fetchRepos", () => { it("returns repos for a user account via paginate.iterator", async () => { const octokit = makeBasicOctokit(); const result = await fetchRepos( - octokit as unknown as ReturnType, + octokit as never, "octocat", "user" ); @@ -205,7 +206,7 @@ describe("fetchIssues", () => { it("returns issues from GraphQL search results", async () => { const octokit = makeIssueOctokit(); const result = await fetchIssues( - octokit as unknown as ReturnType, + octokit as never, [testRepo], "octocat" ); @@ -218,7 +219,7 @@ describe("fetchIssues", () => { it("uses GraphQL search with involves qualifier", async () => { const octokit = makeIssueOctokit(); await fetchIssues( - octokit as unknown as ReturnType, + octokit as never, [testRepo], "octocat" ); @@ -236,7 +237,7 @@ describe("fetchIssues", () => { it("includes repo qualifiers in GraphQL search query", async () => { const octokit = makeIssueOctokit(); await fetchIssues( - octokit as unknown as ReturnType, + octokit as never, [testRepo], "octocat" ); @@ -250,7 +251,7 @@ describe("fetchIssues", () => { it("maps GraphQL fields to camelCase issue shape", async () => { const octokit = makeIssueOctokit(); const { issues } = await fetchIssues( - octokit as unknown as ReturnType, + octokit as never, [testRepo], "octocat" ); @@ -269,7 +270,7 @@ describe("fetchIssues", () => { it("returns empty result when repos is empty", async () => { const octokit = makeIssueOctokit(); const result = await fetchIssues( - octokit as unknown as ReturnType, + octokit as never, [], "octocat" ); @@ -288,7 +289,7 @@ describe("fetchIssues", () => { const octokit = makeIssueOctokit(); await fetchIssues( - octokit as unknown as ReturnType, + octokit as never, repos, "octocat" ); @@ -307,7 +308,7 @@ describe("fetchIssues", () => { // Both batches return the same databaseId const octokit = makeIssueOctokit(async () => makeGraphqlIssueResponse([graphqlIssueNode])); const result = await fetchIssues( - octokit as unknown as ReturnType, + octokit as never, repos, "octocat" ); @@ -330,7 +331,7 @@ describe("fetchIssues", () => { }); const result = await fetchIssues( - octokit as unknown as ReturnType, + octokit as never, [testRepo], "octocat" ); @@ -347,7 +348,7 @@ describe("fetchIssues", () => { const octokit = makeIssueOctokit(async () => makeGraphqlIssueResponse(manyNodes, true, 1500)); const result = await fetchIssues( - octokit as unknown as ReturnType, + octokit as never, [testRepo], "octocat" ); @@ -375,7 +376,7 @@ describe("fetchIssues", () => { })); const result = await fetchIssues( - octokit as unknown as ReturnType, + octokit as never, [testRepo], "octocat" ); @@ -398,7 +399,7 @@ describe("fetchIssues", () => { }); const result = await fetchIssues( - octokit as unknown as ReturnType, + octokit as never, [testRepo], "octocat" ); @@ -435,7 +436,7 @@ describe("fetchIssues", () => { }); const result = await fetchIssues( - octokit as unknown as ReturnType, + octokit as never, [testRepo], "octocat" ); @@ -458,7 +459,7 @@ describe("fetchIssues", () => { })); const result = await fetchIssues( - octokit as unknown as ReturnType, + octokit as never, [testRepo], "octocat" ); @@ -472,9 +473,9 @@ describe("fetchIssues", () => { it("rejects invalid userLogin with error instead of injecting into query", async () => { const octokit = makeIssueOctokit(); const result = await fetchIssues( - octokit as unknown as ReturnType, + octokit as never, [testRepo], - "bad user" // contains space — fails VALID_LOGIN + "bad user" // contains space — fails VALID_TRACKED_LOGIN ); expect(result.issues).toEqual([]); @@ -505,7 +506,7 @@ describe("fetchIssues", () => { }); const result = await fetchIssues( - octokit as unknown as ReturnType, + octokit as never, repos, "octocat" ); @@ -583,7 +584,7 @@ describe("fetchPullRequests", () => { }); await fetchPullRequests( - octokit as unknown as ReturnType, + octokit as never, [testRepo], "octocat" ); @@ -597,7 +598,7 @@ describe("fetchPullRequests", () => { const octokit = makePROctokit(); const { pullRequests } = await fetchPullRequests( - octokit as unknown as ReturnType, + octokit as never, [testRepo], "octocat" ); @@ -638,7 +639,7 @@ describe("fetchPullRequests", () => { } as typeof graphqlPRNode; const octokit = makePROctokit(async () => makeGraphqlPRResponse([node])); const { pullRequests } = await fetchPullRequests( - octokit as unknown as ReturnType, + octokit as never, [testRepo], "octocat" ); @@ -656,7 +657,7 @@ describe("fetchPullRequests", () => { const octokit = makePROctokit(async () => makeGraphqlPRResponse([nodeWithOverlap])); const { pullRequests } = await fetchPullRequests( - octokit as unknown as ReturnType, + octokit as never, [testRepo], "octocat" ); @@ -672,7 +673,7 @@ describe("fetchPullRequests", () => { const node = { ...graphqlPRNode, databaseId: 300, reviewDecision: decision } as typeof graphqlPRNode; const octokit = makePROctokit(async () => makeGraphqlPRResponse([node])); const { pullRequests } = await fetchPullRequests( - octokit as unknown as ReturnType, + octokit as never, [testRepo], "octocat" ); @@ -685,7 +686,7 @@ describe("fetchPullRequests", () => { const octokit = makePROctokit(async () => makeGraphqlPRResponse([graphqlPRNode])); const { pullRequests } = await fetchPullRequests( - octokit as unknown as ReturnType, + octokit as never, [testRepo], "octocat" ); @@ -719,7 +720,7 @@ describe("fetchPullRequests", () => { }); const { pullRequests } = await fetchPullRequests( - octokit as unknown as ReturnType, + octokit as never, [testRepo], "octocat" ); @@ -744,7 +745,7 @@ describe("fetchPullRequests", () => { }); const { pullRequests } = await fetchPullRequests( - octokit as unknown as ReturnType, + octokit as never, [testRepo], "octocat" ); @@ -767,7 +768,7 @@ describe("fetchPullRequests", () => { const octokit = makePROctokit(async () => makeGraphqlPRResponse([malformedNode])); const { pullRequests } = await fetchPullRequests( - octokit as unknown as ReturnType, + octokit as never, [testRepo], "octocat" ); @@ -793,7 +794,7 @@ describe("fetchPullRequests", () => { }); const { pullRequests } = await fetchPullRequests( - octokit as unknown as ReturnType, + octokit as never, [testRepo], "octocat" ); @@ -830,7 +831,7 @@ describe("fetchPullRequests", () => { }); const { pullRequests } = await fetchPullRequests( - octokit as unknown as ReturnType, + octokit as never, [testRepo], "octocat" ); @@ -880,7 +881,7 @@ describe("fetchPullRequests", () => { }); const { pullRequests } = await fetchPullRequests( - octokit as unknown as ReturnType, + octokit as never, [testRepo], "octocat" ); @@ -924,7 +925,7 @@ describe("fetchPullRequests", () => { }); const result = await fetchPullRequests( - octokit as unknown as ReturnType, + octokit as never, [testRepo], "octocat" ); @@ -961,7 +962,7 @@ describe("fetchPullRequests", () => { }); const result = await fetchPullRequests( - octokit as unknown as ReturnType, + octokit as never, [testRepo], "octocat" ); @@ -981,7 +982,7 @@ describe("fetchPullRequests", () => { })); const result = await fetchPullRequests( - octokit as unknown as ReturnType, + octokit as never, [testRepo], "octocat" ); @@ -1004,7 +1005,7 @@ describe("fetchPullRequests", () => { }); const result = await fetchPullRequests( - octokit as unknown as ReturnType, + octokit as never, [testRepo], "octocat" ); @@ -1050,7 +1051,7 @@ describe("fetchPullRequests", () => { }); const { pullRequests } = await fetchPullRequests( - octokit as unknown as ReturnType, + octokit as never, [testRepo], "octocat" ); @@ -1066,7 +1067,7 @@ describe("fetchPullRequests", () => { it("returns empty result when repos is empty", async () => { const octokit = makePROctokit(); const result = await fetchPullRequests( - octokit as unknown as ReturnType, + octokit as never, [], "octocat" ); @@ -1105,7 +1106,7 @@ describe("fetchWorkflowRuns", () => { const octokit = makeOctokitForRuns(); const { workflowRuns } = await fetchWorkflowRuns( - octokit as unknown as ReturnType, + octokit as never, [testRepo], 5, 3 @@ -1119,7 +1120,7 @@ describe("fetchWorkflowRuns", () => { const octokit = makeOctokitForRuns(); await fetchWorkflowRuns( - octokit as unknown as ReturnType, + octokit as never, [testRepo], 5, 3 @@ -1142,7 +1143,7 @@ describe("fetchWorkflowRuns", () => { const maxWorkflows = 3; const maxRuns = 1; const { workflowRuns } = await fetchWorkflowRuns( - octokit as unknown as ReturnType, + octokit as never, [testRepo], maxWorkflows, maxRuns @@ -1163,7 +1164,7 @@ describe("fetchWorkflowRuns", () => { const maxWorkflows = 1; const { workflowRuns } = await fetchWorkflowRuns( - octokit as unknown as ReturnType, + octokit as never, [testRepo], maxWorkflows, 10 @@ -1178,7 +1179,7 @@ describe("fetchWorkflowRuns", () => { const octokit = makeOctokitForRuns(); const { workflowRuns } = await fetchWorkflowRuns( - octokit as unknown as ReturnType, + octokit as never, [testRepo], 5, 10 @@ -1199,7 +1200,7 @@ describe("fetchWorkflowRuns", () => { const octokit = makeOctokitForRuns(); const { workflowRuns } = await fetchWorkflowRuns( - octokit as unknown as ReturnType, + octokit as never, [testRepo], 5, 3 @@ -1227,7 +1228,7 @@ describe("fetchWorkflowRuns", () => { const octokit = makeOctokitForRuns(); const { workflowRuns } = await fetchWorkflowRuns( - octokit as unknown as ReturnType, + octokit as never, [testRepo], 5, 10 @@ -1245,7 +1246,7 @@ describe("fetchWorkflowRuns", () => { const octokit = makeOctokitForRuns(); const { workflowRuns } = await fetchWorkflowRuns( - octokit as unknown as ReturnType, + octokit as never, [testRepo], 5, 10 @@ -1268,7 +1269,7 @@ describe("fetchWorkflowRuns", () => { const octokit = makeOctokitForRuns(); const { workflowRuns } = await fetchWorkflowRuns( - octokit as unknown as ReturnType, + octokit as never, [testRepo], 5, 10 @@ -1285,7 +1286,7 @@ describe("fetchWorkflowRuns", () => { const octokit = makeOctokitForRuns(); const { workflowRuns } = await fetchWorkflowRuns( - octokit as unknown as ReturnType, + octokit as never, [testRepo], 5, 10 @@ -1308,7 +1309,7 @@ describe("empty userLogin short-circuit", () => { }; const result = await fetchIssues( - octokit as unknown as ReturnType, + octokit as never, [testRepo], "" // empty userLogin ); @@ -1324,7 +1325,7 @@ describe("empty userLogin short-circuit", () => { const octokit = { request, graphql, paginate: { iterator: vi.fn() } }; const result = await fetchPullRequests( - octokit as unknown as ReturnType, + octokit as never, [testRepo], "" // empty userLogin ); @@ -1378,7 +1379,7 @@ describe("fetchWorkflowRuns pagination", () => { }; const { workflowRuns } = await fetchWorkflowRuns( - octokit as unknown as ReturnType, + octokit as never, [testRepo], 5, 25 @@ -1413,7 +1414,7 @@ describe("fetchWorkflowRuns pagination", () => { }; await fetchWorkflowRuns( - octokit as unknown as ReturnType, + octokit as never, [testRepo], 5, 3 @@ -1423,3 +1424,635 @@ describe("fetchWorkflowRuns pagination", () => { expect(requestCount).toBe(1); }); }); + +// ── validateGitHubUser — bot login and type detection (C1) ─────────────────── + +function makeUserOctokit(userData: { + login: string; + avatar_url: string; + name: string | null; + type: string; +}) { + return makeOctokit(async () => ({ data: userData })); +} + +describe("validateGitHubUser — VALID_TRACKED_LOGIN and type detection", () => { + it("accepts regular user login", async () => { + const octokit = makeUserOctokit({ + login: "octocat", + avatar_url: "https://avatars.githubusercontent.com/u/583231", + name: "The Octocat", + type: "User", + }); + const result = await validateGitHubUser( + octokit as never, + "octocat" + ); + expect(result).not.toBeNull(); + expect(result?.type).toBe("user"); + }); + + it("accepts bot login with [bot] suffix", async () => { + const octokit = makeUserOctokit({ + login: "dependabot[bot]", + avatar_url: "https://avatars.githubusercontent.com/u/27347476", + name: null, + type: "Bot", + }); + const result = await validateGitHubUser( + octokit as never, + "dependabot[bot]" + ); + expect(result).not.toBeNull(); + expect(result?.type).toBe("bot"); + expect(result?.login).toBe("dependabot[bot]"); + }); + + it("accepts another bot login — khepri-bot[bot]", async () => { + const octokit = makeUserOctokit({ + login: "khepri-bot[bot]", + avatar_url: "https://avatars.githubusercontent.com/u/999", + name: null, + type: "Bot", + }); + const result = await validateGitHubUser( + octokit as never, + "khepri-bot[bot]" + ); + expect(result).not.toBeNull(); + expect(result?.type).toBe("bot"); + }); + + it("returns type:user when API returns type:User", async () => { + const octokit = makeUserOctokit({ + login: "regular-user", + avatar_url: "https://avatars.githubusercontent.com/u/1", + name: "Regular User", + type: "User", + }); + const result = await validateGitHubUser( + octokit as never, + "regular-user" + ); + expect(result?.type).toBe("user"); + }); + + it("returns null for [bot] alone (no base login)", async () => { + const octokit = makeOctokit(async () => ({ data: {} })); + const result = await validateGitHubUser( + octokit as never, + "[bot]" + ); + expect(result).toBeNull(); + }); + + it("returns null for login with arbitrary bracket content", async () => { + const octokit = makeOctokit(async () => ({ data: {} })); + const result = await validateGitHubUser( + octokit as never, + "user[evil]" + ); + expect(result).toBeNull(); + }); + + it("returns null for [Bot] (case-sensitive — only [bot] accepted)", async () => { + const octokit = makeOctokit(async () => ({ data: {} })); + const result = await validateGitHubUser( + octokit as never, + "user[Bot]" + ); + expect(result).toBeNull(); + }); + + it("returns null for user[bot][bot] (double suffix)", async () => { + const octokit = makeOctokit(async () => ({ data: {} })); + const result = await validateGitHubUser( + octokit as never, + "user[bot][bot]" + ); + expect(result).toBeNull(); + }); + + it("returns null on 404 for bot login", async () => { + const octokit = makeOctokit(async () => { + const err = Object.assign(new Error("Not Found"), { status: 404 }); + throw err; + }); + const result = await validateGitHubUser( + octokit as never, + "nonexistent[bot]" + ); + expect(result).toBeNull(); + }); + + it("throws on network error", async () => { + const octokit = makeOctokit(async () => { + throw new Error("Network error"); + }); + await expect( + validateGitHubUser( + octokit as never, + "dependabot[bot]" + ) + ).rejects.toThrow("Network error"); + }); +}); + +// ── fetchIssuesAndPullRequests — monitoredRepos partition (C5) ──────────────── + +describe("fetchIssuesAndPullRequests — monitoredRepos", () => { + it("returns empty result with no repos even when monitoredRepos provided", async () => { + const { fetchIssuesAndPullRequests } = await import("../../src/app/services/api"); + const octokit = makeOctokit(async () => ({ data: {} }), async () => ({})); + const result = await fetchIssuesAndPullRequests( + octokit as never, + [], + "octocat", + undefined, + undefined, + [{ owner: "org", name: "monitored", fullName: "org/monitored" }] + ); + expect(result.issues).toEqual([]); + expect(result.pullRequests).toEqual([]); + }); + + it("unfiltered search query does not contain 'involves:'", async () => { + const queriesUsed: string[] = []; + const octokit = makeOctokit( + async () => ({ data: {} }), + async (_query: string, variables: unknown) => { + const vars = variables as Record; + if (vars.issueQ) queriesUsed.push(vars.issueQ as string); + if (vars.prQ) queriesUsed.push(vars.prQ as string); + return { + issues: { issueCount: 0, pageInfo: { hasNextPage: false, endCursor: null }, nodes: [] }, + prs: { issueCount: 0, pageInfo: { hasNextPage: false, endCursor: null }, nodes: [] }, + // Also handle regular combined search format + prInvolves: { issueCount: 0, pageInfo: { hasNextPage: false, endCursor: null }, nodes: [] }, + prReviewReq: { issueCount: 0, pageInfo: { hasNextPage: false, endCursor: null }, nodes: [] }, + rateLimit: { limit: 5000, remaining: 4999, resetAt: new Date(Date.now() + 3600000).toISOString() }, + }; + } + ); + + const { fetchIssuesAndPullRequests } = await import("../../src/app/services/api"); + const monitoredRepo = { owner: "org", name: "monitored", fullName: "org/monitored" }; + await fetchIssuesAndPullRequests( + octokit as never, + [monitoredRepo], + "octocat", + undefined, + undefined, + [{ owner: "org", name: "monitored", fullName: "org/monitored" }] + ); + + // Unfiltered queries should not contain 'involves:' + const unfilteredQueries = queriesUsed.filter(q => !q.includes("involves:") && !q.includes("review-requested:")); + expect(unfilteredQueries.length).toBeGreaterThan(0); + for (const q of unfilteredQueries) { + expect(q).not.toContain("involves:"); + } + }); +}); + +describe("fetchIssuesAndPullRequests — all repos monitored (edge case)", () => { + it("skips main user light search and returns items from unfiltered search only", async () => { + const queriesUsed: string[] = []; + const repo = { owner: "org", name: "repo1", fullName: "org/repo1" }; + + const issueNode = { + databaseId: 3001, + number: 1, + title: "All-monitored issue", + state: "open", + url: "https://github.com/org/repo1/issues/1", + createdAt: "2024-01-01T00:00:00Z", + updatedAt: "2024-01-02T00:00:00Z", + author: { login: "someone", avatarUrl: "https://avatars.githubusercontent.com/u/1" }, + labels: { nodes: [] }, + assignees: { nodes: [] }, + repository: { nameWithOwner: "org/repo1" }, + comments: { totalCount: 0 }, + }; + + const octokit = makeOctokit( + async () => ({ data: {}, headers: {} }), + async (_query: string, variables: unknown) => { + const vars = variables as Record; + if (vars.issueQ) queriesUsed.push(vars.issueQ as string); + return { + issues: { issueCount: 1, pageInfo: { hasNextPage: false, endCursor: null }, nodes: [issueNode] }, + prs: { 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: new Date(Date.now() + 3600000).toISOString() }, + }; + } + ); + + const { fetchIssuesAndPullRequests } = await import("../../src/app/services/api"); + const result = await fetchIssuesAndPullRequests( + octokit as never, + [repo], + "octocat", + undefined, + undefined, + [{ owner: "org", name: "repo1", fullName: "org/repo1" }] // all repos monitored + ); + + // Items returned from unfiltered search + expect(result.issues).toHaveLength(1); + expect(result.issues[0].id).toBe(3001); + + // No involves: query (main user search skipped since normalRepos is empty) + const involvesQueries = queriesUsed.filter(q => q.includes("involves:octocat")); + expect(involvesQueries).toHaveLength(0); + + // Unfiltered query was issued (no involves: qualifier) + const unfilteredQueries = queriesUsed.filter(q => !q.includes("involves:") && !q.includes("review-requested:")); + expect(unfilteredQueries.length).toBeGreaterThan(0); + + // Item has no surfacedBy (unfiltered search items aren't attributed to a user) + expect(result.issues[0].surfacedBy).toBeUndefined(); + }); +}); + +// ── fetchIssuesAndPullRequests — cross-feature integration (monitored + bot) ── + +describe("fetchIssuesAndPullRequests — cross-feature: monitored repo + bot tracked user", () => { + const monitoredRepo = { owner: "org", name: "monitored", fullName: "org/monitored" }; + const normalRepo = { owner: "org", name: "normal", fullName: "org/normal" }; + + const monitoredIssueNode = { + databaseId: 2001, + number: 1, + title: "Monitored repo issue", + state: "open", + url: "https://github.com/org/monitored/issues/1", + createdAt: "2024-01-01T00:00:00Z", + updatedAt: "2024-01-02T00:00:00Z", + author: { login: "someone", avatarUrl: "https://avatars.githubusercontent.com/u/1" }, + labels: { nodes: [] }, + assignees: { nodes: [] }, + repository: { nameWithOwner: "org/monitored" }, + comments: { totalCount: 0 }, + }; + + const botIssueNode = { + databaseId: 2002, + number: 2, + title: "Bot-surfaced issue", + state: "open", + url: "https://github.com/org/normal/issues/2", + createdAt: "2024-01-01T00:00:00Z", + updatedAt: "2024-01-02T00:00:00Z", + author: { login: "dependabot[bot]", avatarUrl: "https://avatars.githubusercontent.com/u/27347476" }, + labels: { nodes: [] }, + assignees: { nodes: [] }, + repository: { nameWithOwner: "org/normal" }, + comments: { totalCount: 0 }, + }; + + it("deduplicates issues when same item appears in both monitored and bot searches", async () => { + const { fetchIssuesAndPullRequests } = await import("../../src/app/services/api"); + + const octokit = makeOctokit( + async () => ({ data: {}, headers: {} }), + async (_query: string, variables: unknown) => { + const vars = variables as Record; + // Unfiltered search for monitored repo returns monitoredIssueNode + if (vars.issueQ && typeof vars.issueQ === "string" && !String(vars.issueQ).includes("involves:")) { + return { + issues: { issueCount: 1, pageInfo: { hasNextPage: false, endCursor: null }, nodes: [monitoredIssueNode] }, + prs: { issueCount: 0, pageInfo: { hasNextPage: false, endCursor: null }, nodes: [] }, + rateLimit: { limit: 5000, remaining: 4990, resetAt: new Date(Date.now() + 3600000).toISOString() }, + }; + } + // Bot tracked user search on normal repos returns botIssueNode + if (vars.issueQ && String(vars.issueQ).includes("involves:dependabot")) { + return { + issues: { issueCount: 1, pageInfo: { hasNextPage: false, endCursor: null }, nodes: [botIssueNode] }, + prInvolves: { issueCount: 0, pageInfo: { hasNextPage: false, endCursor: null }, nodes: [] }, + prReviewReq: { issueCount: 0, pageInfo: { hasNextPage: false, endCursor: null }, nodes: [] }, + rateLimit: { limit: 5000, remaining: 4985, resetAt: new Date(Date.now() + 3600000).toISOString() }, + }; + } + // Main user search returns nothing + return { + 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: new Date(Date.now() + 3600000).toISOString() }, + }; + } + ); + + const botUser = { login: "dependabot[bot]", avatarUrl: "https://avatars.githubusercontent.com/u/27347476", name: null, type: "bot" as const }; + const result = await fetchIssuesAndPullRequests( + octokit as never, + [normalRepo, monitoredRepo], + "octocat", + undefined, + [botUser], + [{ owner: "org", name: "monitored", fullName: "org/monitored" }] + ); + + // Both issues are present (no dedup since different IDs) + const ids = result.issues.map((i) => i.id); + expect(ids).toContain(2001); // monitored repo issue + expect(ids).toContain(2002); // bot-surfaced issue + + // Bot issue has surfacedBy annotation + const botIssue = result.issues.find((i) => i.id === 2002); + expect(botIssue?.surfacedBy).toContain("dependabot[bot]"); + + // Monitored repo issue has no surfacedBy (no user qualifier) + const monitoredIssue = result.issues.find((i) => i.id === 2001); + expect(monitoredIssue?.surfacedBy).toBeUndefined(); + }); +}); + +// ── VALID_TRACKED_LOGIN — GraphQL injection character rejection ──────────────── + +describe("VALID_TRACKED_LOGIN — rejects GraphQL-unsafe characters", () => { + it("rejects login with space", async () => { + const octokit = makeOctokit(async () => ({ data: {} })); + const result = await validateGitHubUser(octokit as never, "user name"); + expect(result).toBeNull(); + expect(octokit.request).not.toHaveBeenCalled(); + }); + + it("rejects login with colon", async () => { + const octokit = makeOctokit(async () => ({ data: {} })); + const result = await validateGitHubUser(octokit as never, "user:name"); + expect(result).toBeNull(); + expect(octokit.request).not.toHaveBeenCalled(); + }); + + it("rejects login with double quote", async () => { + const octokit = makeOctokit(async () => ({ data: {} })); + const result = await validateGitHubUser(octokit as never, 'user"name'); + expect(result).toBeNull(); + expect(octokit.request).not.toHaveBeenCalled(); + }); + + it("rejects login with newline", async () => { + const octokit = makeOctokit(async () => ({ data: {} })); + const result = await validateGitHubUser(octokit as never, "user\nname"); + expect(result).toBeNull(); + expect(octokit.request).not.toHaveBeenCalled(); + }); +}); + +describe("fetchIssuesAndPullRequests — unfiltered search error handling", () => { + it("returns partial results when unfiltered GraphQL query throws with partial data", async () => { + const monitoredRepo = { owner: "org", name: "monitored", fullName: "org/monitored" }; + const issueNode = { + databaseId: 4001, + number: 1, + title: "Partial issue", + state: "open", + url: "https://github.com/org/monitored/issues/1", + createdAt: "2024-01-01T00:00:00Z", + updatedAt: "2024-01-02T00:00:00Z", + author: { login: "someone", avatarUrl: "https://avatars.githubusercontent.com/u/1" }, + labels: { nodes: [] }, + assignees: { nodes: [] }, + repository: { nameWithOwner: "org/monitored" }, + comments: { totalCount: 0 }, + }; + + let callCount = 0; + const octokit = makeOctokit( + async () => ({ data: {}, headers: {} }), + async () => { + callCount++; + // First call is for unfiltered search — throw with partial data + if (callCount === 1) { + const err = Object.assign(new Error("Partial failure"), { + data: { + issues: { issueCount: 1, pageInfo: { hasNextPage: false, endCursor: null }, nodes: [issueNode] }, + prs: null, + rateLimit: { limit: 5000, remaining: 4998, resetAt: new Date(Date.now() + 3600000).toISOString() }, + }, + }); + throw err; + } + // No other searches expected (normalRepos is empty) + return { + issues: { issueCount: 0, pageInfo: { hasNextPage: false, endCursor: null }, nodes: [] }, + prs: { 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: new Date(Date.now() + 3600000).toISOString() }, + }; + } + ); + + const { fetchIssuesAndPullRequests } = await import("../../src/app/services/api"); + const result = await fetchIssuesAndPullRequests( + octokit as never, + [monitoredRepo], + "octocat", + undefined, + undefined, + [{ owner: "org", name: "monitored", fullName: "org/monitored" }] + ); + + // Partial issue data recovered + expect(result.issues).toHaveLength(1); + expect(result.issues[0].id).toBe(4001); + // Error recorded + expect(result.errors.some(e => e.retryable)).toBe(true); + }); + + it("records error when unfiltered GraphQL query throws with no data", async () => { + const monitoredRepo = { owner: "org", name: "monitored", fullName: "org/monitored" }; + + let callCount = 0; + const octokit = makeOctokit( + async () => ({ data: {}, headers: {} }), + async () => { + callCount++; + if (callCount === 1) { + throw new Error("Total failure"); + } + return { + issues: { issueCount: 0, pageInfo: { hasNextPage: false, endCursor: null }, nodes: [] }, + prs: { 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: new Date(Date.now() + 3600000).toISOString() }, + }; + } + ); + + const { fetchIssuesAndPullRequests } = await import("../../src/app/services/api"); + const result = await fetchIssuesAndPullRequests( + octokit as never, + [monitoredRepo], + "octocat", + undefined, + undefined, + [{ owner: "org", name: "monitored", fullName: "org/monitored" }] + ); + + // No results recovered + expect(result.issues).toHaveLength(0); + expect(result.pullRequests).toHaveLength(0); + // Error recorded with retryable flag (network error → statusCode null → retryable) + expect(result.errors.length).toBeGreaterThan(0); + expect(result.errors[0]).toMatchObject({ retryable: true }); + }); + + it("recovers PR data when issues is null but prs has data (symmetric partial case)", async () => { + const monitoredRepo = { owner: "org", name: "monitored", fullName: "org/monitored" }; + // Fixture matches GraphQLLightPRNode — no heavy fields (additions, commits, etc.) + const prNode = { + id: "PR_kwDOtest4501", + databaseId: 4501, + number: 1, + title: "Partial PR", + state: "open", + isDraft: false, + url: "https://github.com/org/monitored/pull/1", + createdAt: "2024-01-01T00:00:00Z", + updatedAt: "2024-01-02T00:00:00Z", + author: { login: "someone", avatarUrl: "https://avatars.githubusercontent.com/u/1" }, + repository: { nameWithOwner: "org/monitored" }, + headRefName: "fix/thing", + baseRefName: "main", + reviewDecision: null, + labels: { nodes: [] }, + }; + + let callCount = 0; + const octokit = makeOctokit( + async () => ({ data: {}, headers: {} }), + async () => { + callCount++; + if (callCount === 1) { + const err = Object.assign(new Error("Partial failure"), { + data: { + issues: null, + prs: { issueCount: 1, pageInfo: { hasNextPage: false, endCursor: null }, nodes: [prNode] }, + rateLimit: { limit: 5000, remaining: 4998, resetAt: new Date(Date.now() + 3600000).toISOString() }, + }, + }); + throw err; + } + return { + issues: { issueCount: 0, pageInfo: { hasNextPage: false, endCursor: null }, nodes: [] }, + prs: { 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: new Date(Date.now() + 3600000).toISOString() }, + }; + } + ); + + const { fetchIssuesAndPullRequests } = await import("../../src/app/services/api"); + const result = await fetchIssuesAndPullRequests( + octokit as never, + [monitoredRepo], + "octocat", + undefined, + undefined, + [{ owner: "org", name: "monitored", fullName: "org/monitored" }] + ); + + // PR data recovered from partial response + expect(result.pullRequests).toHaveLength(1); + expect(result.pullRequests[0].id).toBe(4501); + // Issues empty (null coalesced to empty) + expect(result.issues).toHaveLength(0); + // Error recorded + expect(result.errors.some(e => e.retryable)).toBe(true); + }); +}); + +describe("fetchIssuesAndPullRequests — all monitored + tracked users skips involves: search", () => { + it("does not fire tracked user involves: search when all repos are monitored", async () => { + const queriesUsed: string[] = []; + const repo = { owner: "org", name: "repo1", fullName: "org/repo1" }; + const botUser = { login: "dependabot[bot]", avatarUrl: "https://avatars.githubusercontent.com/u/27347476", name: null, type: "bot" as const }; + + const octokit = makeOctokit( + async () => ({ data: {}, headers: {} }), + async (_query: string, variables: unknown) => { + const vars = variables as Record; + if (vars.issueQ) queriesUsed.push(vars.issueQ as string); + if (vars.prInvQ) queriesUsed.push(vars.prInvQ as string); + return { + issues: { issueCount: 0, pageInfo: { hasNextPage: false, endCursor: null }, nodes: [] }, + prs: { 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: new Date(Date.now() + 3600000).toISOString() }, + }; + } + ); + + const { fetchIssuesAndPullRequests } = await import("../../src/app/services/api"); + await fetchIssuesAndPullRequests( + octokit as never, + [repo], + "octocat", + undefined, + [botUser], + [{ owner: "org", name: "repo1", fullName: "org/repo1" }] // all repos monitored + ); + + // No involves: queries for tracked user (since normalRepos is empty, tracked search is skipped) + const involvesQueries = queriesUsed.filter(q => q.includes("involves:dependabot")); + expect(involvesQueries).toHaveLength(0); + }); +}); + +describe("fetchIssuesAndPullRequests — onLightData suppression when all monitored", () => { + it("does not call onLightData when all repos are monitored", async () => { + const repo = { owner: "org", name: "repo1", fullName: "org/repo1" }; + const issueNode = { + databaseId: 5001, + number: 1, + title: "Monitored issue", + state: "open", + url: "https://github.com/org/repo1/issues/1", + createdAt: "2024-01-01T00:00:00Z", + updatedAt: "2024-01-02T00:00:00Z", + author: { login: "someone", avatarUrl: "https://avatars.githubusercontent.com/u/1" }, + labels: { nodes: [] }, + assignees: { nodes: [] }, + repository: { nameWithOwner: "org/repo1" }, + comments: { totalCount: 0 }, + }; + + const octokit = makeOctokit( + async () => ({ data: {}, headers: {} }), + async () => ({ + issues: { issueCount: 1, pageInfo: { hasNextPage: false, endCursor: null }, nodes: [issueNode] }, + prs: { 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: new Date(Date.now() + 3600000).toISOString() }, + }) + ); + + const onLightData = vi.fn(); + const { fetchIssuesAndPullRequests } = await import("../../src/app/services/api"); + const result = await fetchIssuesAndPullRequests( + octokit as never, + [repo], + "octocat", + onLightData, + undefined, + [{ owner: "org", name: "repo1", fullName: "org/repo1" }] // all repos monitored + ); + + // onLightData NOT called (main user search skipped, unfiltered results come after) + expect(onLightData).not.toHaveBeenCalled(); + // But final result still has the issue + expect(result.issues).toHaveLength(1); + expect(result.issues[0].id).toBe(5001); + }); +}); diff --git a/tests/services/hot-poll.test.ts b/tests/services/hot-poll.test.ts index 84cde603..36cfabeb 100644 --- a/tests/services/hot-poll.test.ts +++ b/tests/services/hot-poll.test.ts @@ -206,7 +206,7 @@ describe("resetPollState", () => { it("clears hot sets and resets generation", async () => { rebuildHotSets({ ...emptyData, - pullRequests: [makePullRequest({ id: 1, checkStatus: "pending", nodeId: "PR_x" })], + pullRequests: [makePullRequest({ id: 1, checkStatus: "pending", enriched: true, nodeId: "PR_x" })], workflowRuns: [makeWorkflowRun({ id: 10, status: "in_progress", conclusion: null, repoFullName: "o/r" })], }); expect(getHotPollGeneration()).toBe(1); @@ -235,7 +235,7 @@ describe("rebuildHotSets", () => { expect(getHotPollGeneration()).toBe(2); }); - it("populates hot PRs for pending/null checkStatus with nodeId", async () => { + it("populates hot PRs for enriched pending checkStatus with nodeId", async () => { const octokit = makeOctokit(undefined, () => Promise.resolve({ nodes: [], rateLimit: { limit: 5000, remaining: 4999, resetAt: "2026-01-01T00:00:00Z" }, @@ -245,20 +245,20 @@ describe("rebuildHotSets", () => { rebuildHotSets({ ...emptyData, pullRequests: [ - makePullRequest({ id: 1, checkStatus: "pending", nodeId: "PR_a" }), - makePullRequest({ id: 2, checkStatus: null, nodeId: "PR_b" }), - makePullRequest({ id: 3, checkStatus: "success", nodeId: "PR_c" }), // should be skipped - makePullRequest({ id: 4, checkStatus: "pending" }), // no nodeId, should be skipped + makePullRequest({ id: 1, checkStatus: "pending", enriched: true, nodeId: "PR_a" }), + makePullRequest({ id: 2, checkStatus: null, enriched: true, nodeId: "PR_b" }), // null checkStatus — skipped (not pending) + makePullRequest({ id: 3, checkStatus: "success", enriched: true, nodeId: "PR_c" }), // resolved — skipped + makePullRequest({ id: 4, checkStatus: "pending", enriched: true }), // no nodeId — skipped + makePullRequest({ id: 5, checkStatus: "pending", enriched: false, nodeId: "PR_e" }), // not enriched — skipped ], }); await fetchHotData(); - // Verify graphql was called with only the 2 eligible node IDs + // Verify graphql was called with only the 1 eligible node ID expect(octokit.graphql).toHaveBeenCalledTimes(1); const calledIds = (octokit.graphql.mock.calls[0][1] as { ids: string[] }).ids; - expect(calledIds).toHaveLength(2); + expect(calledIds).toHaveLength(1); expect(calledIds).toContain("PR_a"); - expect(calledIds).toContain("PR_b"); }); it("populates hot runs for queued/in_progress, skips completed", async () => { @@ -377,7 +377,7 @@ describe("fetchHotData", () => { rebuildHotSets({ ...emptyData, - pullRequests: [makePullRequest({ id: 1, checkStatus: "pending", nodeId: "PR_x" })], + pullRequests: [makePullRequest({ id: 1, checkStatus: "pending", enriched: true, nodeId: "PR_x" })], }); // First fetch — PR is hot, returns success -> evicts @@ -603,7 +603,7 @@ describe("createHotPollCoordinator", () => { rebuildHotSets({ ...emptyData, - pullRequests: [makePullRequest({ id: 1, checkStatus: "pending", nodeId: "PR_a" })], + pullRequests: [makePullRequest({ id: 1, checkStatus: "pending", enriched: true, nodeId: "PR_a" })], }); const { pushError } = await import("../../src/app/lib/errors"); @@ -661,7 +661,7 @@ describe("createHotPollCoordinator", () => { rebuildHotSets({ ...emptyData, - pullRequests: [makePullRequest({ id: 42, nodeId: "PR_node42", repoFullName: "o/r" })], + pullRequests: [makePullRequest({ id: 42, checkStatus: "pending", enriched: true, nodeId: "PR_node42", repoFullName: "o/r" })], workflowRuns: [makeWorkflowRun({ id: 7, status: "in_progress", conclusion: null, repoFullName: "o/r" })], }); @@ -746,7 +746,7 @@ describe("createHotPollCoordinator", () => { rebuildHotSets({ ...emptyData, - pullRequests: [makePullRequest({ id: 42, nodeId: "PR_node42", repoFullName: "o/r" })], + pullRequests: [makePullRequest({ id: 42, checkStatus: "pending", enriched: true, nodeId: "PR_node42", repoFullName: "o/r" })], }); const { pushError } = await import("../../src/app/lib/errors"); @@ -868,7 +868,7 @@ describe("rebuildHotSets caps", () => { it("caps hot PRs at MAX_HOT_PRS (200)", async () => { const prs = Array.from({ length: 250 }, (_, i) => - makePullRequest({ id: i + 1, checkStatus: "pending", nodeId: `PR_${i}` }) + makePullRequest({ id: i + 1, checkStatus: "pending", enriched: true, nodeId: `PR_${i}` }) ); const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); @@ -945,8 +945,8 @@ describe("fetchHotData eviction edge cases", () => { rebuildHotSets({ ...emptyData, pullRequests: [ - makePullRequest({ id: 1, checkStatus: "pending", nodeId: "PR_one" }), - makePullRequest({ id: 2, checkStatus: "pending", nodeId: "PR_two" }), + makePullRequest({ id: 1, checkStatus: "pending", enriched: true, nodeId: "PR_one" }), + makePullRequest({ id: 2, checkStatus: "pending", enriched: true, nodeId: "PR_two" }), ], }); @@ -979,7 +979,7 @@ describe("fetchHotData eviction edge cases", () => { rebuildHotSets({ ...emptyData, - pullRequests: [makePullRequest({ id: 1, checkStatus: "pending", nodeId: "PR_merged" })], + pullRequests: [makePullRequest({ id: 1, checkStatus: "pending", enriched: true, nodeId: "PR_merged" })], }); const first = await fetchHotData(); @@ -1008,7 +1008,7 @@ describe("fetchHotData eviction edge cases", () => { rebuildHotSets({ ...emptyData, - pullRequests: [makePullRequest({ id: 2, checkStatus: null, nodeId: "PR_closed" })], + pullRequests: [makePullRequest({ id: 2, checkStatus: "pending", enriched: true, nodeId: "PR_closed" })], }); await fetchHotData(); @@ -1022,7 +1022,7 @@ describe("clearHotSets", () => { it("empties both hot maps so next fetchHotData is a no-op", async () => { rebuildHotSets({ ...emptyData, - pullRequests: [makePullRequest({ id: 1, checkStatus: "pending", nodeId: "PR_a" })], + pullRequests: [makePullRequest({ id: 1, checkStatus: "pending", enriched: true, nodeId: "PR_a" })], workflowRuns: [makeWorkflowRun({ id: 10, status: "in_progress", conclusion: null, repoFullName: "o/r" })], }); @@ -1060,7 +1060,7 @@ describe("fetchHotData hadErrors", () => { rebuildHotSets({ ...emptyData, - pullRequests: [makePullRequest({ id: 1, checkStatus: "pending", nodeId: "PR_a" })], + pullRequests: [makePullRequest({ id: 1, checkStatus: "pending", enriched: true, nodeId: "PR_a" })], workflowRuns: [makeWorkflowRun({ id: 10, status: "in_progress", conclusion: null, repoFullName: "o/r" })], }); @@ -1074,7 +1074,7 @@ describe("fetchHotData hadErrors", () => { rebuildHotSets({ ...emptyData, - pullRequests: [makePullRequest({ id: 1, checkStatus: "pending", nodeId: "PR_a" })], + pullRequests: [makePullRequest({ id: 1, checkStatus: "pending", enriched: true, nodeId: "PR_a" })], }); const { hadErrors, prUpdates } = await fetchHotData(); diff --git a/tests/services/poll-fetchAllData.test.ts b/tests/services/poll-fetchAllData.test.ts index 246b771f..26ca5aaf 100644 --- a/tests/services/poll-fetchAllData.test.ts +++ b/tests/services/poll-fetchAllData.test.ts @@ -136,7 +136,7 @@ describe("fetchAllData — first call", () => { await fetchAllData(); - expect(fetchIssuesAndPullRequests).toHaveBeenCalledWith(mockOctokit, config.selectedRepos, "octocat", undefined, []); + expect(fetchIssuesAndPullRequests).toHaveBeenCalledWith(mockOctokit, config.selectedRepos, "octocat", undefined, [], []); expect(fetchWorkflowRuns).toHaveBeenCalledWith( mockOctokit, config.selectedRepos, @@ -719,6 +719,7 @@ describe("fetchAllData — upstream repos and tracked users", () => { selectedRepos, "octocat", undefined, + [], [] ); expect(fetchWorkflowRuns).toHaveBeenCalledWith( diff --git a/tests/services/poll-notification-effects.test.ts b/tests/services/poll-notification-effects.test.ts new file mode 100644 index 00000000..495193d4 --- /dev/null +++ b/tests/services/poll-notification-effects.test.ts @@ -0,0 +1,197 @@ +/** + * Tests for the module-scope reactive effects in poll.ts that reset notification + * state when config.trackedUsers or config.monitoredRepos change. + * + * Uses the REAL reactive config store (not a static mock) so that updateConfig() + * triggers the reactive effects registered by poll.ts at module load. + */ +import "fake-indexeddb/auto"; +import { describe, it, expect, vi, beforeEach } from "vitest"; + +const { mockResetNotifState } = vi.hoisted(() => ({ + mockResetNotifState: vi.fn(), +})); + +// Mock github client +vi.mock("../../src/app/services/github", () => ({ + getClient: vi.fn(), +})); + +// Mock auth store — onAuthCleared is called at poll.ts module scope +vi.mock("../../src/app/stores/auth", () => ({ + user: vi.fn(() => ({ login: "octocat", avatar_url: "https://github.com/images/error/octocat_happy.gif", name: "Octocat" })), + onAuthCleared: vi.fn(), +})); + +// Mock API functions +vi.mock("../../src/app/services/api", () => ({ + fetchIssuesAndPullRequests: vi.fn(), + fetchWorkflowRuns: vi.fn(), + fetchHotPRStatus: vi.fn(), + fetchWorkflowRunById: vi.fn(), + pooledAllSettled: vi.fn(), + resetEmptyActionRepos: vi.fn(), +})); + +// Mock notifications — spy on _resetNotificationState +vi.mock("../../src/app/lib/notifications", () => ({ + detectNewItems: vi.fn(() => []), + dispatchNotifications: vi.fn(), + _resetNotificationState: mockResetNotifState, +})); + +// Mock errors store +vi.mock("../../src/app/lib/errors", () => ({ + pushError: vi.fn(), + pushNotification: vi.fn(), + getErrors: vi.fn().mockReturnValue([]), + dismissError: vi.fn(), + getNotifications: vi.fn().mockReturnValue([]), + getUnreadCount: vi.fn().mockReturnValue(0), + markAllAsRead: vi.fn(), + startCycleTracking: vi.fn(), + endCycleTracking: vi.fn(), + resetNotificationState: vi.fn(), + dismissNotificationBySource: vi.fn(), +})); + +// Use REAL config store — the reactive effects in poll.ts subscribe to this +import { updateConfig, resetConfig } from "../../src/app/stores/config"; + +// Import poll.ts — triggers createRoot + createEffect registration at module scope +import { fetchAllData, resetPollState } from "../../src/app/services/poll"; +import { getClient } from "../../src/app/services/github"; +import { fetchIssuesAndPullRequests, fetchWorkflowRuns } from "../../src/app/services/api"; + +describe("poll.ts — notification reset reactive effects", () => { + beforeEach(() => { + resetConfig(); + mockResetNotifState.mockClear(); + }); + + it("resets notification state when monitoredRepos changes", () => { + updateConfig({ + selectedRepos: [{ owner: "org", name: "repo", fullName: "org/repo" }], + monitoredRepos: [{ owner: "org", name: "repo", fullName: "org/repo" }], + }); + + expect(mockResetNotifState).toHaveBeenCalled(); + }); + + it("resets notification state when trackedUsers changes", () => { + updateConfig({ + trackedUsers: [{ + login: "octocat", + avatarUrl: "https://avatars.githubusercontent.com/u/583231", + name: "Octocat", + type: "user" as const, + }], + }); + + expect(mockResetNotifState).toHaveBeenCalled(); + }); + + it("does not reset when config update does not change the key", () => { + updateConfig({ theme: "dark" }); + + expect(mockResetNotifState).not.toHaveBeenCalled(); + }); + + it("resets notification state when monitoredRepos cleared to empty", () => { + updateConfig({ + selectedRepos: [{ owner: "org", name: "repo", fullName: "org/repo" }], + monitoredRepos: [{ owner: "org", name: "repo", fullName: "org/repo" }], + }); + mockResetNotifState.mockClear(); + + updateConfig({ monitoredRepos: [] }); + + expect(mockResetNotifState).toHaveBeenCalled(); + }); + + it("detects swap at same array length (key-based comparison)", () => { + updateConfig({ + selectedRepos: [ + { owner: "org", name: "a", fullName: "org/a" }, + { owner: "org", name: "b", fullName: "org/b" }, + ], + monitoredRepos: [{ owner: "org", name: "a", fullName: "org/a" }], + }); + mockResetNotifState.mockClear(); + + updateConfig({ + monitoredRepos: [{ owner: "org", name: "b", fullName: "org/b" }], + }); + + expect(mockResetNotifState).toHaveBeenCalled(); + }); +}); + +describe("poll.ts — notifications gate bypass on config change", () => { + const mockRequest = vi.fn(); + + beforeEach(() => { + resetPollState(); + resetConfig(); + mockRequest.mockReset(); + mockRequest.mockResolvedValue({ + data: [], + headers: { "last-modified": "Thu, 20 Mar 2026 12:00:00 GMT" }, + }); + vi.mocked(getClient).mockReturnValue({ + request: mockRequest, + graphql: vi.fn(), + hook: { before: vi.fn() }, + } as never); + vi.mocked(fetchIssuesAndPullRequests).mockResolvedValue({ + issues: [], pullRequests: [], errors: [], + }); + vi.mocked(fetchWorkflowRuns).mockResolvedValue({ + workflowRuns: [], errors: [], + } as never); + }); + + it("bypasses notifications gate after monitoredRepos change", async () => { + // First call — no _lastSuccessfulFetch, gate skipped + await fetchAllData(); + expect(mockRequest).not.toHaveBeenCalled(); + + // Second call — _lastSuccessfulFetch set, gate fires + await fetchAllData(); + expect(mockRequest).toHaveBeenCalledWith("GET /notifications", expect.anything()); + mockRequest.mockClear(); + + // Change monitoredRepos — should null _lastSuccessfulFetch + updateConfig({ + selectedRepos: [{ owner: "org", name: "repo", fullName: "org/repo" }], + monitoredRepos: [{ owner: "org", name: "repo", fullName: "org/repo" }], + }); + + // Third call — gate bypassed because _lastSuccessfulFetch was nulled + await fetchAllData(); + expect(mockRequest).not.toHaveBeenCalled(); + }); + + it("bypasses notifications gate after trackedUsers change", async () => { + // First call — sets _lastSuccessfulFetch + await fetchAllData(); + + // Second call — gate fires + await fetchAllData(); + mockRequest.mockClear(); + + // Change trackedUsers — should null _lastSuccessfulFetch + updateConfig({ + trackedUsers: [{ + login: "octocat", + avatarUrl: "https://avatars.githubusercontent.com/u/583231", + name: "Octocat", + type: "user" as const, + }], + }); + + // Next call — gate bypassed + await fetchAllData(); + expect(mockRequest).not.toHaveBeenCalled(); + }); +}); diff --git a/tests/stores/auth.test.ts b/tests/stores/auth.test.ts index 41ccb265..9205d157 100644 --- a/tests/stores/auth.test.ts +++ b/tests/stores/auth.test.ts @@ -5,6 +5,11 @@ vi.mock("../../src/app/stores/cache", () => ({ clearCache: vi.fn().mockResolvedValue(undefined), })); +const mockPushNotification = vi.fn(); +vi.mock("../../src/app/lib/errors", () => ({ + pushNotification: (...args: unknown[]) => mockPushNotification(...args), +})); + // Mock localStorage with full control (same pattern as config.test.ts) const localStorageMock = (() => { let store: Record = {}; @@ -68,6 +73,24 @@ describe("setAuth / token signal", () => { const fresh = await import("../../src/app/stores/auth"); expect(fresh.token()).toBe("ghs_persisted"); }); + + it("sets token in memory and warns when localStorage.setItem throws", () => { + const origSetItem = localStorageMock.setItem; + localStorageMock.setItem = () => { throw new DOMException("QuotaExceededError"); }; + mockPushNotification.mockClear(); + + try { + mod.setAuth({ access_token: "ghs_quota" }); + expect(mod.token()).toBe("ghs_quota"); + expect(mockPushNotification).toHaveBeenCalledWith( + "localStorage:auth", + expect.stringContaining("storage may be full"), + "warning", + ); + } finally { + localStorageMock.setItem = origSetItem; + } + }); }); describe("isAuthenticated", () => { @@ -131,6 +154,58 @@ describe("clearAuth", () => { }); }); +describe("expireToken", () => { + let mod: typeof import("../../src/app/stores/auth"); + + beforeEach(async () => { + localStorageMock.clear(); + vi.resetModules(); + mod = await import("../../src/app/stores/auth"); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("clears token signal to null", () => { + mod.setAuth({ access_token: "ghs_abc" }); + mod.expireToken(); + expect(mod.token()).toBeNull(); + }); + + it("clears user signal to null", () => { + // Simulate a validated session: set token + manually set user via validateToken mock + vi.stubGlobal("fetch", vi.fn().mockResolvedValue({ + ok: true, status: 200, + json: () => Promise.resolve({ login: "wgordon", avatar_url: "", name: "Will" }), + })); + mod.setAuth({ access_token: "ghs_abc" }); + // Directly test that user is cleared — validateToken would set it, but + // we can verify the signal reset by checking after expireToken + mod.expireToken(); + expect(mod.user()).toBeNull(); + }); + + it("removes only auth token from localStorage — config, view, dashboard preserved", () => { + localStorageMock.setItem("github-tracker:config", '{"theme":"dark"}'); + localStorageMock.setItem("github-tracker:view", '{"lastActiveTab":"actions"}'); + localStorageMock.setItem("github-tracker:dashboard", '{"cached":true}'); + mod.setAuth({ access_token: "ghs_abc" }); + mod.expireToken(); + expect(localStorageMock.getItem("github-tracker:auth-token")).toBeNull(); + expect(localStorageMock.getItem("github-tracker:config")).toBe('{"theme":"dark"}'); + expect(localStorageMock.getItem("github-tracker:view")).toBe('{"lastActiveTab":"actions"}'); + expect(localStorageMock.getItem("github-tracker:dashboard")).toBe('{"cached":true}'); + }); + + it("does NOT invoke onAuthCleared callbacks", () => { + const cb = vi.fn(); + mod.onAuthCleared(cb); + mod.expireToken(); + expect(cb).not.toHaveBeenCalled(); + }); +}); + describe("onAuthCleared callbacks", () => { let mod: typeof import("../../src/app/stores/auth"); @@ -201,12 +276,14 @@ describe("validateToken", () => { let mod: typeof import("../../src/app/stores/auth"); beforeEach(async () => { + vi.useFakeTimers(); localStorageMock.clear(); vi.resetModules(); mod = await import("../../src/app/stores/auth"); }); afterEach(() => { + vi.useRealTimers(); vi.restoreAllMocks(); vi.unstubAllGlobals(); }); @@ -231,16 +308,110 @@ describe("validateToken", () => { expect(mod.user()?.name).toBe("Will"); }); - it("clears auth on 401 (permanent token revoked — no refresh fallback)", async () => { + it("expires token after two consecutive 401s (transient retry + genuine revocation)", async () => { vi.stubGlobal("fetch", vi.fn() - // GET /user returns 401 (access token revoked) + // First GET /user returns 401 + .mockResolvedValueOnce({ ok: false, status: 401, json: () => Promise.resolve({}) }) + // Retry GET /user also returns 401 .mockResolvedValueOnce({ ok: false, status: 401, json: () => Promise.resolve({}) }) ); mod.setAuth({ access_token: "ghs_revoked" }); - await mod.validateToken(); + const promise = mod.validateToken(); + await vi.advanceTimersByTimeAsync(1000); + await promise; + expect(mod.token()).toBeNull(); + expect(localStorageMock.getItem("github-tracker:auth-token")).toBeNull(); + }); + + it("recovers on transient 401 when retry succeeds", async () => { + vi.stubGlobal("fetch", vi.fn() + // First GET /user returns 401 (transient) + .mockResolvedValueOnce({ ok: false, status: 401, json: () => Promise.resolve({}) }) + // Retry succeeds + .mockResolvedValueOnce({ + ok: true, status: 200, + json: () => Promise.resolve({ login: "wgordon", avatar_url: "", name: "Will" }), + }) + ); + + mod.setAuth({ access_token: "ghs_transient" }); + const promise = mod.validateToken(); + await vi.advanceTimersByTimeAsync(1000); + const result = await promise; + expect(result).toBe(true); + expect(mod.token()).toBe("ghs_transient"); + expect(mod.user()?.login).toBe("wgordon"); + }); + + it("preserves token when retry returns non-401 error (e.g. 500)", async () => { + vi.stubGlobal("fetch", vi.fn() + .mockResolvedValueOnce({ ok: false, status: 401, json: () => Promise.resolve({}) }) + .mockResolvedValueOnce({ ok: false, status: 500, json: () => Promise.resolve({}) }) + ); + + mod.setAuth({ access_token: "ghs_retry500" }); + const promise = mod.validateToken(); + await vi.advanceTimersByTimeAsync(1000); + const result = await promise; + expect(result).toBe(false); + // Token preserved — server error on retry doesn't confirm revocation + expect(mod.token()).toBe("ghs_retry500"); + }); + + it("preserves token when retry throws network error", async () => { + vi.stubGlobal("fetch", vi.fn() + .mockResolvedValueOnce({ ok: false, status: 401, json: () => Promise.resolve({}) }) + .mockRejectedValueOnce(new TypeError("Failed to fetch")) + ); + + mod.setAuth({ access_token: "ghs_retrynet" }); + const promise = mod.validateToken(); + await vi.advanceTimersByTimeAsync(1000); + const result = await promise; + expect(result).toBe(false); + // Token preserved — network error on retry doesn't confirm revocation + expect(mod.token()).toBe("ghs_retrynet"); + }); + + it("does not invalidate a new token set during the retry window (race condition guard)", async () => { + vi.stubGlobal("fetch", vi.fn() + // First GET /user returns 401 + .mockResolvedValueOnce({ ok: false, status: 401, json: () => Promise.resolve({}) }) + // Retry also returns 401 — but token was replaced mid-window + .mockResolvedValueOnce({ ok: false, status: 401, json: () => Promise.resolve({}) }) + ); + + mod.setAuth({ access_token: "ghs_old" }); + const promise = mod.validateToken(); + // Simulate user re-authenticating during the 1-second retry delay + mod.setAuth({ access_token: "ghs_new" }); + await vi.advanceTimersByTimeAsync(1000); + const result = await promise; + expect(result).toBe(false); + // The NEW token must survive — guard prevents expireToken() from running + expect(mod.token()).toBe("ghs_new"); + expect(localStorageMock.getItem("github-tracker:auth-token")).toBe("ghs_new"); + }); + + it("preserves user config and view state when token expires (not a full logout)", async () => { + vi.stubGlobal("fetch", vi.fn() + .mockResolvedValueOnce({ ok: false, status: 401, json: () => Promise.resolve({}) }) + .mockResolvedValueOnce({ ok: false, status: 401, json: () => Promise.resolve({}) }) + ); + + localStorageMock.setItem("github-tracker:config", '{"theme":"dark"}'); + localStorageMock.setItem("github-tracker:view", '{"lastActiveTab":"actions"}'); + mod.setAuth({ access_token: "ghs_expired" }); + const promise = mod.validateToken(); + await vi.advanceTimersByTimeAsync(1000); + await promise; + // Token cleared expect(mod.token()).toBeNull(); expect(localStorageMock.getItem("github-tracker:auth-token")).toBeNull(); + // Config and view preserved — expireToken() does NOT wipe user data + expect(localStorageMock.getItem("github-tracker:config")).toBe('{"theme":"dark"}'); + expect(localStorageMock.getItem("github-tracker:view")).toBe('{"lastActiveTab":"actions"}'); }); it("returns false and leaves token unchanged on non-200/non-401 response (e.g., 503)", async () => { diff --git a/tests/stores/config.test.ts b/tests/stores/config.test.ts index 898788c8..d1e212b0 100644 --- a/tests/stores/config.test.ts +++ b/tests/stores/config.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, beforeEach } from "vitest"; -import { ConfigSchema, loadConfig, config, updateConfig, resetConfig } from "../../src/app/stores/config"; +import { ConfigSchema, TrackedUserSchema, loadConfig, config, updateConfig, resetConfig, setMonitoredRepo } from "../../src/app/stores/config"; import { createRoot } from "solid-js"; import { createStore, produce } from "solid-js/store"; @@ -160,6 +160,58 @@ describe("ConfigSchema — upstream repos and tracked users", () => { }); expect(result.upstreamRepos).toHaveLength(1); }); + + it("TrackedUserSchema defaults type to 'user' when omitted", () => { + const result = TrackedUserSchema.parse({ + login: "octocat", + avatarUrl: "https://avatars.githubusercontent.com/u/583231", + name: null, + }); + expect(result.type).toBe("user"); + }); + + it("TrackedUserSchema accepts type:'bot'", () => { + const result = TrackedUserSchema.parse({ + login: "dependabot[bot]", + avatarUrl: "https://avatars.githubusercontent.com/u/27347476", + name: null, + type: "bot", + }); + expect(result.type).toBe("bot"); + }); + + it("TrackedUserSchema accepts type:'user'", () => { + const result = TrackedUserSchema.parse({ + login: "octocat", + avatarUrl: "https://avatars.githubusercontent.com/u/583231", + name: null, + type: "user", + }); + expect(result.type).toBe("user"); + }); + + it("TrackedUserSchema rejects invalid type value", () => { + expect(() => TrackedUserSchema.parse({ + login: "octocat", + avatarUrl: "https://avatars.githubusercontent.com/u/583231", + name: null, + type: "organization", + })).toThrow(); + }); + + it("ConfigSchema preserves tracked user type through round-trip", () => { + const result = ConfigSchema.parse({ + trackedUsers: [ + { + login: "dependabot[bot]", + avatarUrl: "https://avatars.githubusercontent.com/u/27347476", + name: null, + type: "bot", + }, + ], + }); + expect(result.trackedUsers[0].type).toBe("bot"); + }); }); describe("loadConfig", () => { @@ -323,7 +375,7 @@ describe("updateConfig (real export)", () => { ["selectedOrgs", { selectedOrgs: ["new-org"] }], ["selectedRepos", { selectedRepos: [{ owner: "x", name: "y", fullName: "x/y" }] }], ["upstreamRepos", { upstreamRepos: [{ owner: "u", name: "v", fullName: "u/v" }] }], - ["trackedUsers", { trackedUsers: [{ login: "bob", avatarUrl: "https://avatars.githubusercontent.com/u/1", name: null }] }], + ["trackedUsers", { trackedUsers: [{ login: "bob", avatarUrl: "https://avatars.githubusercontent.com/u/1", name: null, type: "user" as const }] }], ["refreshInterval", { refreshInterval: 120 }], ["hotPollInterval", { hotPollInterval: 60 }], ["maxWorkflowsPerRepo", { maxWorkflowsPerRepo: 10 }], @@ -343,7 +395,7 @@ describe("updateConfig (real export)", () => { selectedOrgs: ["seed-org"], selectedRepos: [{ owner: "s", name: "r", fullName: "s/r" }], upstreamRepos: [{ owner: "a", name: "b", fullName: "a/b" }], - trackedUsers: [{ login: "alice", avatarUrl: "https://avatars.githubusercontent.com/u/2", name: "Alice" }], + trackedUsers: [{ login: "alice", avatarUrl: "https://avatars.githubusercontent.com/u/2", name: "Alice", type: "user" as const }], refreshInterval: 600, hotPollInterval: 45, maxWorkflowsPerRepo: 8, @@ -373,6 +425,8 @@ describe("updateConfig (real export)", () => { // Every OTHER field must be identical to the snapshot for (const key of Object.keys(before)) { if (key === fieldName) continue; + // monitoredRepos is pruned when selectedRepos changes — skip the cross-check for that case + if (fieldName === "selectedRepos" && key === "monitoredRepos") continue; expect( (config as Record)[key], `updateConfig({ ${fieldName} }) must not change ${key}` @@ -382,3 +436,170 @@ describe("updateConfig (real export)", () => { }); }); }); + +// ── monitoredRepos config schema + setMonitoredRepo (C3) ───────────────────── + +describe("ConfigSchema — monitoredRepos", () => { + it("defaults monitoredRepos to empty array", () => { + const result = ConfigSchema.parse({}); + expect(result.monitoredRepos).toEqual([]); + }); + + it("accepts valid monitoredRepos", () => { + const result = ConfigSchema.parse({ + monitoredRepos: [{ owner: "org", name: "repo", fullName: "org/repo" }], + }); + expect(result.monitoredRepos).toHaveLength(1); + expect(result.monitoredRepos[0].fullName).toBe("org/repo"); + }); +}); + +describe("updateConfig — monitoredRepos pruning on selectedRepos change", () => { + beforeEach(() => { + resetConfig(); + }); + + it("prunes monitoredRepos when selectedRepos removes a repo", () => { + createRoot((dispose) => { + updateConfig({ + selectedRepos: [ + { owner: "org", name: "a", fullName: "org/a" }, + { owner: "org", name: "b", fullName: "org/b" }, + ], + monitoredRepos: [ + { owner: "org", name: "a", fullName: "org/a" }, + { owner: "org", name: "b", fullName: "org/b" }, + ], + }); + // Remove org/b from selectedRepos + updateConfig({ + selectedRepos: [{ owner: "org", name: "a", fullName: "org/a" }], + }); + expect(config.monitoredRepos).toHaveLength(1); + expect(config.monitoredRepos[0].fullName).toBe("org/a"); + dispose(); + }); + }); + + it("does not prune monitoredRepos when selectedRepos is not in the update", () => { + createRoot((dispose) => { + updateConfig({ + selectedRepos: [{ owner: "org", name: "a", fullName: "org/a" }], + monitoredRepos: [{ owner: "org", name: "a", fullName: "org/a" }], + }); + // Update theme only + updateConfig({ theme: "dark" }); + expect(config.monitoredRepos).toHaveLength(1); + dispose(); + }); + }); +}); + +describe("setMonitoredRepo (C3)", () => { + beforeEach(() => { + resetConfig(); + }); + + it("adds a repo to monitoredRepos when monitored=true and repo is in selectedRepos", () => { + createRoot((dispose) => { + updateConfig({ + selectedRepos: [{ owner: "org", name: "a", fullName: "org/a" }], + }); + setMonitoredRepo({ owner: "org", name: "a", fullName: "org/a" }, true); + expect(config.monitoredRepos).toHaveLength(1); + expect(config.monitoredRepos[0].fullName).toBe("org/a"); + dispose(); + }); + }); + + it("is idempotent — adding same repo twice results in one entry", () => { + createRoot((dispose) => { + updateConfig({ + selectedRepos: [{ owner: "org", name: "a", fullName: "org/a" }], + }); + setMonitoredRepo({ owner: "org", name: "a", fullName: "org/a" }, true); + setMonitoredRepo({ owner: "org", name: "a", fullName: "org/a" }, true); + expect(config.monitoredRepos).toHaveLength(1); + dispose(); + }); + }); + + it("removes a repo from monitoredRepos when monitored=false", () => { + createRoot((dispose) => { + updateConfig({ + selectedRepos: [ + { owner: "org", name: "a", fullName: "org/a" }, + { owner: "org", name: "b", fullName: "org/b" }, + ], + monitoredRepos: [ + { owner: "org", name: "a", fullName: "org/a" }, + { owner: "org", name: "b", fullName: "org/b" }, + ], + }); + setMonitoredRepo({ owner: "org", name: "a", fullName: "org/a" }, false); + expect(config.monitoredRepos).toHaveLength(1); + expect(config.monitoredRepos[0].fullName).toBe("org/b"); + dispose(); + }); + }); + + it("is idempotent — removing non-monitored repo is a no-op", () => { + createRoot((dispose) => { + updateConfig({ + selectedRepos: [{ owner: "org", name: "a", fullName: "org/a" }], + monitoredRepos: [], + }); + setMonitoredRepo({ owner: "org", name: "a", fullName: "org/a" }, false); + expect(config.monitoredRepos).toHaveLength(0); + dispose(); + }); + }); + + it("rejects repo not in selectedRepos — does not add to monitoredRepos", () => { + createRoot((dispose) => { + updateConfig({ + selectedRepos: [{ owner: "org", name: "a", fullName: "org/a" }], + }); + setMonitoredRepo({ owner: "org", name: "notselected", fullName: "org/notselected" }, true); + expect(config.monitoredRepos).toHaveLength(0); + dispose(); + }); + }); + + it("enforces max 10 monitored repos", () => { + createRoot((dispose) => { + const repos = Array.from({ length: 11 }, (_, i) => ({ + owner: "org", + name: `repo-${i}`, + fullName: `org/repo-${i}`, + })); + updateConfig({ selectedRepos: repos, monitoredRepos: repos.slice(0, 10) }); + // 10 is the max — adding an 11th should be rejected + setMonitoredRepo(repos[10], true); + expect(config.monitoredRepos).toHaveLength(10); + dispose(); + }); + }); +}); + +describe("ConfigSchema — monitoredRepos max constraint", () => { + it("rejects more than 10 monitored repos at schema level", () => { + const repos = Array.from({ length: 11 }, (_, i) => ({ + owner: "org", + name: `repo-${i}`, + fullName: `org/repo-${i}`, + })); + const result = ConfigSchema.safeParse({ monitoredRepos: repos }); + expect(result.success).toBe(false); + }); + + it("accepts exactly 10 monitored repos", () => { + const repos = Array.from({ length: 10 }, (_, i) => ({ + owner: "org", + name: `repo-${i}`, + fullName: `org/repo-${i}`, + })); + const result = ConfigSchema.safeParse({ monitoredRepos: repos }); + expect(result.success).toBe(true); + }); +});