From 761b500aa1ef029e32785f0e52e6fe691e8f2250 Mon Sep 17 00:00:00 2001 From: testvalue Date: Mon, 30 Mar 2026 21:17:01 -0400 Subject: [PATCH 01/10] feat: adds bot user tracking and per-repo monitor-all mode --- .../components/dashboard/DashboardPage.tsx | 2 + src/app/components/dashboard/IssuesTab.tsx | 19 +- .../components/dashboard/PullRequestsTab.tsx | 19 +- .../onboarding/OnboardingWizard.tsx | 15 + .../components/onboarding/RepoSelector.tsx | 67 ++- src/app/components/settings/SettingsPage.tsx | 5 +- .../settings/TrackedUsersSection.tsx | 15 +- src/app/services/api.ts | 234 +++++++++- src/app/services/poll.ts | 19 +- src/app/stores/config.ts | 23 + tests/components/dashboard/IssuesTab.test.tsx | 75 +++- .../dashboard/PullRequestsTab.test.tsx | 75 +++- .../onboarding/OnboardingWizard.test.tsx | 38 +- .../onboarding/RepoSelector.test.tsx | 90 ++++ .../components/settings/SettingsPage.test.tsx | 28 ++ .../settings/TrackedUsersSection.test.tsx | 57 +++ tests/services/api-users.test.ts | 4 + tests/services/api.test.ts | 420 +++++++++++++++--- tests/services/poll-fetchAllData.test.ts | 3 +- tests/stores/config.test.ts | 190 +++++++- 20 files changed, 1290 insertions(+), 108 deletions(-) diff --git a/src/app/components/dashboard/DashboardPage.tsx b/src/app/components/dashboard/DashboardPage.tsx index bfc2408a..f35d31dc 100644 --- a/src/app/components/dashboard/DashboardPage.tsx +++ b/src/app/components/dashboard/DashboardPage.tsx @@ -359,6 +359,7 @@ export default function DashboardPage() { userLogin={userLogin()} allUsers={allUsers()} trackedUsers={config.trackedUsers} + monitoredRepos={config.monitoredRepos} /> @@ -369,6 +370,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..5f2ab3ce 100644 --- a/src/app/components/dashboard/IssuesTab.tsx +++ b/src/app/components/dashboard/IssuesTab.tsx @@ -25,6 +25,7 @@ export interface IssuesTabProps { userLogin: string; allUsers?: { login: string; label: string }[]; trackedUsers?: TrackedUser[]; + monitoredRepos?: { fullName: string }[]; } 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..6fd7da5d 100644 --- a/src/app/components/dashboard/PullRequestsTab.tsx +++ b/src/app/components/dashboard/PullRequestsTab.tsx @@ -30,6 +30,7 @@ export interface PullRequestsTabProps { allUsers?: { login: string; label: string }[]; trackedUsers?: TrackedUser[]; hotPollingPRIds?: ReadonlySet; + monitoredRepos?: { fullName: string }[]; } 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..32958250 100644 --- a/src/app/components/onboarding/OnboardingWizard.tsx +++ b/src/app/components/onboarding/OnboardingWizard.tsx @@ -19,6 +19,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); @@ -53,10 +56,14 @@ export default function OnboardingWizard() { function handleFinish() { const uniqueOrgs = [...new Set(selectedRepos().map((r) => r.owner))]; + // Prune monitoredRepos to only repos still in selectedRepos + const selectedSet = new Set(selectedRepos().map((r) => r.fullName)); + const prunedMonitoredRepos = monitoredRepos().filter((r) => selectedSet.has(r.fullName)); updateConfig({ selectedOrgs: uniqueOrgs, selectedRepos: selectedRepos(), upstreamRepos: upstreamRepos(), + monitoredRepos: prunedMonitoredRepos, onboardingComplete: true, }); // Flush synchronously — the debounced persistence effect won't fire before page unload @@ -116,6 +123,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..a8741ba7 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,48 @@ 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..394cf252 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 { 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"; @@ -142,6 +142,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, @@ -324,6 +325,8 @@ export default function SettingsPage() { upstreamRepos={localUpstream()} onUpstreamChange={handleUpstreamChange} trackedUsers={config.trackedUsers} + monitoredRepos={config.monitoredRepos} + onMonitorToggle={(repo, monitored) => setMonitoredRepo(repo, monitored)} /> diff --git a/src/app/components/settings/TrackedUsersSection.tsx b/src/app/components/settings/TrackedUsersSection.tsx index 9968e7a1..b785c89f 100644 --- a/src/app/components/settings/TrackedUsersSection.tsx +++ b/src/app/components/settings/TrackedUsersSection.tsx @@ -118,11 +118,16 @@ export default function TrackedUsersSection(props: TrackedUsersSectionProps) { {trackedUser.name ?? trackedUser.login} - - - {trackedUser.login} - - +
    + + + {trackedUser.login} + + + + bot + +
    + 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..d131c27e 100644 --- a/tests/components/onboarding/RepoSelector.test.tsx +++ b/tests/components/onboarding/RepoSelector.test.tsx @@ -691,3 +691,93 @@ 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 + ); + }); +}); diff --git a/tests/components/settings/SettingsPage.test.tsx b/tests/components/settings/SettingsPage.test.tsx index 8d96688a..4ad53248 100644 --- a/tests/components/settings/SettingsPage.test.tsx +++ b/tests/components/settings/SettingsPage.test.tsx @@ -682,3 +682,31 @@ describe("SettingsPage — Manage org access button", () => { expect(apiModule.fetchOrgs).toHaveBeenCalledTimes(1); }); }); + +describe("SettingsPage — monitor toggle wiring", () => { + 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/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..c5ee595d 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,7 +473,7 @@ 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 ); @@ -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,320 @@ 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, + [{ 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, + [{ 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:"); + } + }); +}); + +// ── 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], + [{ 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(); + }); +}); 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/stores/config.test.ts b/tests/stores/config.test.ts index 898788c8..62ce5440 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,133 @@ 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(); + }); + }); +}); From 41215bc889d1504c01137114bdab8f6f94158b94 Mon Sep 17 00:00:00 2001 From: testvalue Date: Mon, 30 Mar 2026 21:26:52 -0400 Subject: [PATCH 02/10] fix(poll): uses serialized keys for notification reset --- src/app/services/poll.ts | 40 ++++++++++++------------ tests/services/api.test.ts | 62 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 20 deletions(-) diff --git a/src/app/services/poll.ts b/src/app/services/poll.ts index a2fd6862..aa5f2cf2 100644 --- a/src/app/services/poll.ts +++ b/src/app/services/poll.ts @@ -87,35 +87,35 @@ export function resetPollState(): void { // Auto-reset poll state on logout (avoids circular dep with auth.ts) onAuthCleared(resetPollState); -// When tracked users change, reset notification state so the next poll cycle -// silently seeds all items (including the new tracked user's) without flooding -// the user with "new item" notifications for pre-existing content. -// Use a flag to skip the initial run (module-level mount). -// Wrapped in createRoot to provide a reactive owner at module scope (per SolidJS gotcha). -// Subscribes to array length only — fires on add/remove, not property mutations. -let _trackedUsersMounted = false; +// When tracked users or monitored repos change, reset notification state so the +// next poll cycle silently seeds items without flooding "new item" notifications. +// Tracks a serialized key (sorted logins/fullNames) so swapping entries at the +// same array length still triggers the reset. +let _trackedUsersKey = ""; +let _monitoredReposKey = ""; createRoot(() => { createEffect(() => { - void (config.trackedUsers?.length ?? 0); - if (!_trackedUsersMounted) { - _trackedUsersMounted = true; + const key = (config.trackedUsers ?? []).map((u) => u.login).sort().join(","); + if (_trackedUsersKey === "") { + _trackedUsersKey = key; return; } - untrack(() => _resetNotificationState()); + if (key !== _trackedUsersKey) { + _trackedUsersKey = key; + untrack(() => _resetNotificationState()); + } }); -}); -// When monitoredRepos changes, reset notification state so the next poll cycle -// silently seeds the newly monitored repo's items without flooding with notifications. -let _monitoredReposMounted = false; -createRoot(() => { createEffect(() => { - void (config.monitoredRepos?.length ?? 0); - if (!_monitoredReposMounted) { - _monitoredReposMounted = true; + const key = (config.monitoredRepos ?? []).map((r) => r.fullName).sort().join(","); + if (_monitoredReposKey === "") { + _monitoredReposKey = key; return; } - untrack(() => _resetNotificationState()); + if (key !== _monitoredReposKey) { + _monitoredReposKey = key; + untrack(() => _resetNotificationState()); + } }); }); diff --git a/tests/services/api.test.ts b/tests/services/api.test.ts index c5ee595d..5bc23038 100644 --- a/tests/services/api.test.ts +++ b/tests/services/api.test.ts @@ -1615,6 +1615,68 @@ describe("fetchIssuesAndPullRequests — monitoredRepos", () => { }); }); +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, + [{ 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", () => { From 715d789803ee929f674d8be83286d12bf39ddf63 Mon Sep 17 00:00:00 2001 From: testvalue Date: Mon, 30 Mar 2026 21:33:46 -0400 Subject: [PATCH 03/10] fix(poll): restores boolean mount guard for notification reset --- src/app/services/poll.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/app/services/poll.ts b/src/app/services/poll.ts index aa5f2cf2..ab1f0144 100644 --- a/src/app/services/poll.ts +++ b/src/app/services/poll.ts @@ -90,13 +90,17 @@ onAuthCleared(resetPollState); // When tracked users or monitored repos change, reset notification state so the // next poll cycle silently seeds items without flooding "new item" notifications. // Tracks a serialized key (sorted logins/fullNames) so swapping entries at the -// same array length still triggers the reset. +// same array length still triggers the reset. Boolean mount flags ensure the +// initial effect run is always skipped (key="" is a valid state for empty arrays). +let _trackedUsersMounted = false; let _trackedUsersKey = ""; +let _monitoredReposMounted = false; let _monitoredReposKey = ""; createRoot(() => { createEffect(() => { const key = (config.trackedUsers ?? []).map((u) => u.login).sort().join(","); - if (_trackedUsersKey === "") { + if (!_trackedUsersMounted) { + _trackedUsersMounted = true; _trackedUsersKey = key; return; } @@ -108,7 +112,8 @@ createRoot(() => { createEffect(() => { const key = (config.monitoredRepos ?? []).map((r) => r.fullName).sort().join(","); - if (_monitoredReposKey === "") { + if (!_monitoredReposMounted) { + _monitoredReposMounted = true; _monitoredReposKey = key; return; } From 34a0ed691fa45bdc5723c89bee64b179ce06ea0b Mon Sep 17 00:00:00 2001 From: testvalue Date: Tue, 31 Mar 2026 07:57:31 -0400 Subject: [PATCH 04/10] fix(api): addresses PR #38 review findings - removes trackedSearchRepos fallback that sent involves: queries to monitored repos when all repos were monitored - extracts SEARCH_RESULT_CAP module-level constant (replaces 4 local declarations) - extracts LIGHT_ISSUE_FRAGMENT GraphQL fragment (eliminates 3rd copy of inline issue field selection) - simplifies SettingsPage onMonitorToggle lambda - types monitoredRepos prop as RepoRef[] in tab components - documents poll.ts mount-guard permanence across resetPollState() - adds 6 tests: unfiltered search error handling, all-monitored + tracked users, onLightData suppression, upstream repo toggle guard --- src/app/components/dashboard/IssuesTab.tsx | 4 +- .../components/dashboard/PullRequestsTab.tsx | 4 +- src/app/components/settings/SettingsPage.tsx | 2 +- src/app/services/api.ts | 131 ++++++------ src/app/services/poll.ts | 3 + tests/components/dashboard/IssuesTab.test.tsx | 4 +- .../dashboard/PullRequestsTab.test.tsx | 4 +- .../onboarding/RepoSelector.test.tsx | 18 ++ tests/services/api.test.ts | 187 ++++++++++++++++++ 9 files changed, 274 insertions(+), 83 deletions(-) diff --git a/src/app/components/dashboard/IssuesTab.tsx b/src/app/components/dashboard/IssuesTab.tsx index 5f2ab3ce..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,7 +25,7 @@ export interface IssuesTabProps { userLogin: string; allUsers?: { login: string; label: string }[]; trackedUsers?: TrackedUser[]; - monitoredRepos?: { fullName: string }[]; + monitoredRepos?: RepoRef[]; } type SortField = "repo" | "title" | "author" | "createdAt" | "updatedAt" | "comments"; diff --git a/src/app/components/dashboard/PullRequestsTab.tsx b/src/app/components/dashboard/PullRequestsTab.tsx index 6fd7da5d..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,7 +30,7 @@ export interface PullRequestsTabProps { allUsers?: { login: string; label: string }[]; trackedUsers?: TrackedUser[]; hotPollingPRIds?: ReadonlySet; - monitoredRepos?: { fullName: string }[]; + monitoredRepos?: RepoRef[]; } type SortField = "repo" | "title" | "author" | "createdAt" | "updatedAt" | "checkStatus" | "reviewDecision" | "size"; diff --git a/src/app/components/settings/SettingsPage.tsx b/src/app/components/settings/SettingsPage.tsx index 394cf252..79576474 100644 --- a/src/app/components/settings/SettingsPage.tsx +++ b/src/app/components/settings/SettingsPage.tsx @@ -326,7 +326,7 @@ export default function SettingsPage() { onUpstreamChange={handleUpstreamChange} trackedUsers={config.trackedUsers} monitoredRepos={config.monitoredRepos} - onMonitorToggle={(repo, monitored) => setMonitoredRepo(repo, monitored)} + onMonitorToggle={setMonitoredRepo} /> diff --git a/src/app/services/api.ts b/src/app/services/api.ts index 22945c07..371e8cab 100644 --- a/src/app/services/api.ts +++ b/src/app/services/api.ts @@ -210,6 +210,8 @@ const VALID_REPO_NAME = /^[A-Za-z0-9._-]+\/[A-Za-z0-9._-]+$/; // App bot accounts. Case-sensitive [bot] is intentional — GitHub always uses lowercase. const VALID_TRACKED_LOGIN = /^[A-Za-z0-9-]{1,39}(\[bot\])?$/; +const SEARCH_RESULT_CAP = 1000; + function chunkArray(arr: T[], size: number): T[][] { const chunks: T[][] = []; for (let i = 0; i < arr.length; i += size) { @@ -342,6 +344,23 @@ interface ForkQueryResponse { // ── GraphQL search query constants ─────────────────────────────────────────── +const LIGHT_ISSUE_FRAGMENT = ` + fragment LightIssueFields on Issue { + databaseId + number + title + state + url + createdAt + updatedAt + author { login avatarUrl } + labels(first: 10) { nodes { name color } } + assignees(first: 10) { nodes { login } } + repository { nameWithOwner } + comments { totalCount } + } +`; + const ISSUES_SEARCH_QUERY = ` query($q: String!, $cursor: String) { search(query: $q, type: ISSUE, first: 50, after: $cursor) { @@ -349,23 +368,13 @@ const ISSUES_SEARCH_QUERY = ` pageInfo { hasNextPage endCursor } nodes { ... on Issue { - databaseId - number - title - state - url - createdAt - updatedAt - author { login avatarUrl } - labels(first: 10) { nodes { name color } } - assignees(first: 10) { nodes { login } } - repository { nameWithOwner } - comments { totalCount } + ...LightIssueFields } } } rateLimit { limit remaining resetAt } } + ${LIGHT_ISSUE_FRAGMENT} `; const PR_SEARCH_QUERY = ` @@ -457,18 +466,7 @@ const LIGHT_COMBINED_SEARCH_QUERY = ` pageInfo { hasNextPage endCursor } nodes { ... on Issue { - databaseId - number - title - state - url - createdAt - updatedAt - author { login avatarUrl } - labels(first: 10) { nodes { name color } } - assignees(first: 10) { nodes { login } } - repository { nameWithOwner } - comments { totalCount } + ...LightIssueFields } } } @@ -492,6 +490,7 @@ const LIGHT_COMBINED_SEARCH_QUERY = ` } rateLimit { limit remaining resetAt } } + ${LIGHT_ISSUE_FRAGMENT} ${LIGHT_PR_FRAGMENT} `; @@ -507,18 +506,7 @@ const UNFILTERED_SEARCH_QUERY = ` pageInfo { hasNextPage endCursor } nodes { ... on Issue { - databaseId - number - title - state - url - createdAt - updatedAt - author { login avatarUrl } - labels(first: 10) { nodes { name color } } - assignees(first: 10) { nodes { login } } - repository { nameWithOwner } - comments { totalCount } + ...LightIssueFields } } } @@ -533,6 +521,7 @@ const UNFILTERED_SEARCH_QUERY = ` } rateLimit { limit remaining resetAt } } + ${LIGHT_ISSUE_FRAGMENT} ${LIGHT_PR_FRAGMENT} `; @@ -1095,8 +1084,6 @@ async function graphqlLightCombinedSearch( const prMap = new Map(); const nodeIdMap = new Map(); const errors: ApiError[] = []; - const ISSUE_CAP = 1000; - const PR_CAP = 1000; await Promise.allSettled(chunks.map(async (chunk, chunkIdx) => { const repoQualifiers = buildRepoQualifiers(chunk); @@ -1108,7 +1095,7 @@ async function graphqlLightCombinedSearch( try { await executeLightCombinedQuery( octokit, issueQ, prInvQ, prRevQ, batchLabel, - issueSeen, issues, prMap, nodeIdMap, errors, ISSUE_CAP, PR_CAP, + issueSeen, issues, prMap, nodeIdMap, errors, SEARCH_RESULT_CAP, SEARCH_RESULT_CAP, ); } catch (err) { const { statusCode, message } = extractRejectionError(err); @@ -1116,19 +1103,19 @@ async function graphqlLightCombinedSearch( } })); - if (issues.length >= ISSUE_CAP) { - console.warn(`[api] Issue search results capped at ${ISSUE_CAP}`); + if (issues.length >= SEARCH_RESULT_CAP) { + console.warn(`[api] Issue search results capped at ${SEARCH_RESULT_CAP}`); pushNotification("search/issues", `Issue search results capped at 1,000 — some items are hidden`, "warning"); - issues.splice(ISSUE_CAP); + issues.splice(SEARCH_RESULT_CAP); } - if (prMap.size >= PR_CAP) { - console.warn(`[api] PR search results capped at ${PR_CAP}`); + if (prMap.size >= SEARCH_RESULT_CAP) { + console.warn(`[api] PR search results capped at ${SEARCH_RESULT_CAP}`); pushNotification("search/prs", `PR search results capped at 1,000 — some items are hidden`, "warning"); } const pullRequests = [...prMap.values()]; - if (pullRequests.length >= PR_CAP) pullRequests.splice(PR_CAP); + if (pullRequests.length >= SEARCH_RESULT_CAP) pullRequests.splice(SEARCH_RESULT_CAP); const prNodeIds = pullRequests.map((pr) => nodeIdMap.get(pr.id)).filter((id): id is string => id != null); return { issues, pullRequests, prNodeIds, errors }; @@ -1152,8 +1139,6 @@ async function graphqlUnfilteredSearch( const prMap = new Map(); const nodeIdMap = new Map(); const errors: ApiError[] = []; - const ISSUE_CAP = 1000; - const PR_CAP = 1000; await Promise.allSettled(chunks.map(async (chunk, chunkIdx) => { const repoQualifiers = buildRepoQualifiers(chunk); @@ -1191,31 +1176,31 @@ async function graphqlUnfilteredSearch( if (response.rateLimit) updateGraphqlRateLimit(response.rateLimit); for (const node of response.issues.nodes) { - if (issues.length >= ISSUE_CAP) break; + if (issues.length >= SEARCH_RESULT_CAP) break; if (!node) continue; processIssueNode(node, issueSeen, issues); } for (const node of response.prs.nodes) { - if (prMap.size >= PR_CAP) break; + if (prMap.size >= SEARCH_RESULT_CAP) break; if (!node) continue; processLightPRNode(node, prMap, nodeIdMap); } if (isPartial) return; - if (response.issues.pageInfo.hasNextPage && response.issues.pageInfo.endCursor && issues.length < ISSUE_CAP) { + if (response.issues.pageInfo.hasNextPage && response.issues.pageInfo.endCursor && issues.length < SEARCH_RESULT_CAP) { await paginateGraphQLSearch( octokit, ISSUES_SEARCH_QUERY, issueQ, batchLabel, errors, (node) => processIssueNode(node, issueSeen, issues), - () => issues.length, ISSUE_CAP, response.issues.pageInfo.endCursor, + () => issues.length, SEARCH_RESULT_CAP, response.issues.pageInfo.endCursor, ); } - if (response.prs.pageInfo.hasNextPage && response.prs.pageInfo.endCursor && prMap.size < PR_CAP) { + if (response.prs.pageInfo.hasNextPage && response.prs.pageInfo.endCursor && prMap.size < SEARCH_RESULT_CAP) { await paginateGraphQLSearch( octokit, LIGHT_PR_SEARCH_QUERY, prQ, batchLabel, errors, (node) => processLightPRNode(node, prMap, nodeIdMap), - () => prMap.size, PR_CAP, response.prs.pageInfo.endCursor, + () => prMap.size, SEARCH_RESULT_CAP, response.prs.pageInfo.endCursor, ); } } catch (err) { @@ -1224,19 +1209,19 @@ async function graphqlUnfilteredSearch( } })); - if (issues.length >= ISSUE_CAP) { - console.warn(`[api] Unfiltered issue results capped at ${ISSUE_CAP}`); + if (issues.length >= SEARCH_RESULT_CAP) { + console.warn(`[api] Unfiltered issue results capped at ${SEARCH_RESULT_CAP}`); pushNotification("search/unfiltered-issues", `Monitored repo issue results capped at 1,000 — some items are hidden`, "warning"); - issues.splice(ISSUE_CAP); + issues.splice(SEARCH_RESULT_CAP); } - if (prMap.size >= PR_CAP) { - console.warn(`[api] Unfiltered PR results capped at ${PR_CAP}`); + if (prMap.size >= SEARCH_RESULT_CAP) { + console.warn(`[api] Unfiltered PR results capped at ${SEARCH_RESULT_CAP}`); pushNotification("search/unfiltered-prs", `Monitored repo PR results capped at 1,000 — some items are hidden`, "warning"); } const pullRequests = [...prMap.values()]; - if (pullRequests.length >= PR_CAP) pullRequests.splice(PR_CAP); + if (pullRequests.length >= SEARCH_RESULT_CAP) pullRequests.splice(SEARCH_RESULT_CAP); const prNodeIds = pullRequests.map((pr) => nodeIdMap.get(pr.id)).filter((id): id is string => id != null); return { issues, pullRequests, prNodeIds, errors }; } @@ -1482,11 +1467,11 @@ export async function fetchIssuesAndPullRequests( ? fetchPREnrichment(octokit, mainNodeIds) : Promise.resolve({ enrichments: new Map(), errors: [] as ApiError[] }); - // Tracked user searches — scoped to normalRepos (monitored repos are already covered by - // graphqlUnfilteredSearch, so running involves: on them would only duplicate work) - const trackedSearchRepos = normalRepos.length > 0 ? normalRepos : repos; - const trackedSearchPromise = hasTrackedUsers && repos.length > 0 - ? Promise.allSettled(trackedUsers!.map((u) => graphqlLightCombinedSearch(octokit, trackedSearchRepos, u.login))) + // Tracked user searches — scoped to normalRepos only. Monitored repos are already + // covered by graphqlUnfilteredSearch (all open items, no user qualifier), so running + // involves: on them would duplicate work and add spurious surfacedBy annotations. + const trackedSearchPromise = hasTrackedUsers && normalRepos.length > 0 + ? Promise.allSettled(trackedUsers!.map((u) => graphqlLightCombinedSearch(octokit, normalRepos, u.login))) : Promise.resolve([] as PromiseSettledResult[]); // Unfiltered search for monitored repos — runs in parallel with tracked searches @@ -1590,7 +1575,6 @@ async function graphqlSearchIssues( const seen = new Set(); const issues: Issue[] = []; const errors: ApiError[] = []; - const CAP = 1000; const chunkResults = await Promise.allSettled(chunks.map(async (chunk, chunkIdx) => { const repoQualifiers = buildRepoQualifiers(chunk); @@ -1622,7 +1606,7 @@ async function graphqlSearchIssues( return true; }, () => issues.length, - CAP, + SEARCH_RESULT_CAP, ); })); @@ -1633,10 +1617,10 @@ async function graphqlSearchIssues( } } - if (issues.length >= CAP) { - console.warn(`[api] Issue search results capped at ${CAP}`); + if (issues.length >= SEARCH_RESULT_CAP) { + console.warn(`[api] Issue search results capped at ${SEARCH_RESULT_CAP}`); pushNotification("search/issues", `Issue search results capped at 1,000 — some items are hidden`, "warning"); - issues.splice(CAP); + issues.splice(SEARCH_RESULT_CAP); } return { issues, errors }; @@ -1684,7 +1668,6 @@ async function graphqlSearchPRs( const prMap = new Map(); const forkInfoMap = new Map(); const errors: ApiError[] = []; - const CAP = 1000; function processPRNode(node: GraphQLPRNode): boolean { if (node.databaseId == null || !node.repository) return false; @@ -1761,7 +1744,7 @@ async function graphqlSearchPRs( await paginateGraphQLSearch( octokit, PR_SEARCH_QUERY, queryString, `pr-search-batch-${chunkIdx + 1}/${chunks.length}`, - errors, processPRNode, () => prMap.size, CAP, + errors, processPRNode, () => prMap.size, SEARCH_RESULT_CAP, ); }) ); @@ -1774,8 +1757,8 @@ async function graphqlSearchPRs( } } - if (prMap.size >= CAP) { - console.warn(`[api] PR search results capped at ${CAP}`); + if (prMap.size >= SEARCH_RESULT_CAP) { + console.warn(`[api] PR search results capped at ${SEARCH_RESULT_CAP}`); pushNotification("search/prs", `PR search results capped at 1,000 — some items are hidden`, "warning"); } @@ -1849,7 +1832,7 @@ async function graphqlSearchPRs( } const pullRequests = [...prMap.values()]; - if (pullRequests.length >= CAP) pullRequests.splice(CAP); + if (pullRequests.length >= SEARCH_RESULT_CAP) pullRequests.splice(SEARCH_RESULT_CAP); return { pullRequests, errors }; } diff --git a/src/app/services/poll.ts b/src/app/services/poll.ts index ab1f0144..b90edf9c 100644 --- a/src/app/services/poll.ts +++ b/src/app/services/poll.ts @@ -92,6 +92,9 @@ onAuthCleared(resetPollState); // Tracks a serialized key (sorted logins/fullNames) so swapping entries at the // same array length still triggers the reset. Boolean mount flags ensure the // initial effect run is always skipped (key="" is a valid state for empty arrays). +// NOTE: Mount flags are intentionally permanent (module lifetime) and NOT cleared +// by resetPollState(). The createRoot runs once at module load; the effects +// continue tracking config changes across auth cycles without re-mounting. let _trackedUsersMounted = false; let _trackedUsersKey = ""; let _monitoredReposMounted = false; diff --git a/tests/components/dashboard/IssuesTab.test.tsx b/tests/components/dashboard/IssuesTab.test.tsx index c69db041..f3ffb4e0 100644 --- a/tests/components/dashboard/IssuesTab.test.tsx +++ b/tests/components/dashboard/IssuesTab.test.tsx @@ -254,7 +254,7 @@ describe("IssuesTab — monitored repos filter bypass", () => { issues={issues} userLogin="me" allUsers={[{ login: "me", label: "Me" }, { login: "other-user", label: "other-user" }]} - monitoredRepos={[{ fullName: "org/monitored" }]} + monitoredRepos={[{ owner: "org", name: "monitored", fullName: "org/monitored" }]} /> )); @@ -288,7 +288,7 @@ describe("IssuesTab — monitored repos filter bypass", () => { )); diff --git a/tests/components/dashboard/PullRequestsTab.test.tsx b/tests/components/dashboard/PullRequestsTab.test.tsx index 929117da..39e873e3 100644 --- a/tests/components/dashboard/PullRequestsTab.test.tsx +++ b/tests/components/dashboard/PullRequestsTab.test.tsx @@ -204,7 +204,7 @@ describe("PullRequestsTab — monitored repos filter bypass", () => { pullRequests={prs} userLogin="me" allUsers={[{ login: "me", label: "Me" }, { login: "other-user", label: "other-user" }]} - monitoredRepos={[{ fullName: "org/monitored" }]} + monitoredRepos={[{ owner: "org", name: "monitored", fullName: "org/monitored" }]} /> )); @@ -238,7 +238,7 @@ describe("PullRequestsTab — monitored repos filter bypass", () => { )); diff --git a/tests/components/onboarding/RepoSelector.test.tsx b/tests/components/onboarding/RepoSelector.test.tsx index d131c27e..1ccf2154 100644 --- a/tests/components/onboarding/RepoSelector.test.tsx +++ b/tests/components/onboarding/RepoSelector.test.tsx @@ -780,4 +780,22 @@ describe("RepoSelector — monitor toggle", () => { 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/services/api.test.ts b/tests/services/api.test.ts index 5bc23038..00026e69 100644 --- a/tests/services/api.test.ts +++ b/tests/services/api.test.ts @@ -1803,3 +1803,190 @@ describe("VALID_TRACKED_LOGIN — rejects GraphQL-unsafe characters", () => { 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, + [{ 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, + [{ fullName: "org/monitored" }] + ); + + // No results recovered + expect(result.issues).toHaveLength(0); + expect(result.pullRequests).toHaveLength(0); + // Error recorded + expect(result.errors.length).toBeGreaterThan(0); + }); +}); + +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], + [{ 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, + [{ 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); + }); +}); From a5071fe3942c332028ecf749f181ee527114a984 Mon Sep 17 00:00:00 2001 From: testvalue Date: Tue, 31 Mar 2026 08:08:10 -0400 Subject: [PATCH 05/10] fix(api): addresses Layer 1.5 domain review findings - types fetchIssuesAndPullRequests monitoredRepos param as RepoRef[] - uses SEARCH_RESULT_CAP in pushNotification strings (removes hardcoded 1,000) - removes stale section comment header - tightens test assertion for retryable error - updates test callsites to use full RepoRef objects --- src/app/services/api.ts | 16 +++++++--------- tests/services/api.test.ts | 19 ++++++++++--------- 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/src/app/services/api.ts b/src/app/services/api.ts index 371e8cab..ca370fe5 100644 --- a/src/app/services/api.ts +++ b/src/app/services/api.ts @@ -430,8 +430,6 @@ const PR_SEARCH_QUERY = ` } `; -// ── GraphQL combined search query ───────────────────────────────────────────── - // ── Two-phase rendering: light + heavy queries ─────────────────────────────── const LIGHT_PR_FRAGMENT = ` @@ -1105,13 +1103,13 @@ async function graphqlLightCombinedSearch( if (issues.length >= SEARCH_RESULT_CAP) { console.warn(`[api] Issue search results capped at ${SEARCH_RESULT_CAP}`); - pushNotification("search/issues", `Issue search results capped at 1,000 — some items are hidden`, "warning"); + pushNotification("search/issues", `Issue search results capped at ${SEARCH_RESULT_CAP.toLocaleString()} — some items are hidden`, "warning"); issues.splice(SEARCH_RESULT_CAP); } if (prMap.size >= SEARCH_RESULT_CAP) { console.warn(`[api] PR search results capped at ${SEARCH_RESULT_CAP}`); - pushNotification("search/prs", `PR search results capped at 1,000 — some items are hidden`, "warning"); + pushNotification("search/prs", `PR search results capped at ${SEARCH_RESULT_CAP.toLocaleString()} — some items are hidden`, "warning"); } const pullRequests = [...prMap.values()]; @@ -1211,13 +1209,13 @@ async function graphqlUnfilteredSearch( if (issues.length >= SEARCH_RESULT_CAP) { console.warn(`[api] Unfiltered issue results capped at ${SEARCH_RESULT_CAP}`); - pushNotification("search/unfiltered-issues", `Monitored repo issue results capped at 1,000 — some items are hidden`, "warning"); + pushNotification("search/unfiltered-issues", `Monitored repo issue results capped at ${SEARCH_RESULT_CAP.toLocaleString()} — some items are hidden`, "warning"); issues.splice(SEARCH_RESULT_CAP); } if (prMap.size >= SEARCH_RESULT_CAP) { console.warn(`[api] Unfiltered PR results capped at ${SEARCH_RESULT_CAP}`); - pushNotification("search/unfiltered-prs", `Monitored repo PR results capped at 1,000 — some items are hidden`, "warning"); + pushNotification("search/unfiltered-prs", `Monitored repo PR results capped at ${SEARCH_RESULT_CAP.toLocaleString()} — some items are hidden`, "warning"); } const pullRequests = [...prMap.values()]; @@ -1412,7 +1410,7 @@ export async function fetchIssuesAndPullRequests( userLogin: string, onLightData?: (data: FetchIssuesAndPRsResult) => void, trackedUsers?: TrackedUser[], - monitoredRepos?: { fullName: string }[], + monitoredRepos?: RepoRef[], ): Promise { if (!octokit) throw new Error("No GitHub client available"); @@ -1619,7 +1617,7 @@ async function graphqlSearchIssues( if (issues.length >= SEARCH_RESULT_CAP) { console.warn(`[api] Issue search results capped at ${SEARCH_RESULT_CAP}`); - pushNotification("search/issues", `Issue search results capped at 1,000 — some items are hidden`, "warning"); + pushNotification("search/issues", `Issue search results capped at ${SEARCH_RESULT_CAP.toLocaleString()} — some items are hidden`, "warning"); issues.splice(SEARCH_RESULT_CAP); } @@ -1759,7 +1757,7 @@ async function graphqlSearchPRs( if (prMap.size >= SEARCH_RESULT_CAP) { console.warn(`[api] PR search results capped at ${SEARCH_RESULT_CAP}`); - pushNotification("search/prs", `PR search results capped at 1,000 — some items are hidden`, "warning"); + pushNotification("search/prs", `PR search results capped at ${SEARCH_RESULT_CAP.toLocaleString()} — some items are hidden`, "warning"); } // Fork PR fallback: for PRs with null checkStatus where head repo owner differs from base diff --git a/tests/services/api.test.ts b/tests/services/api.test.ts index 00026e69..16bc1450 100644 --- a/tests/services/api.test.ts +++ b/tests/services/api.test.ts @@ -1570,7 +1570,7 @@ describe("fetchIssuesAndPullRequests — monitoredRepos", () => { "octocat", undefined, undefined, - [{ fullName: "org/monitored" }] + [{ owner: "org", name: "monitored", fullName: "org/monitored" }] ); expect(result.issues).toEqual([]); expect(result.pullRequests).toEqual([]); @@ -1603,7 +1603,7 @@ describe("fetchIssuesAndPullRequests — monitoredRepos", () => { "octocat", undefined, undefined, - [{ fullName: "org/monitored" }] + [{ owner: "org", name: "monitored", fullName: "org/monitored" }] ); // Unfiltered queries should not contain 'involves:' @@ -1657,7 +1657,7 @@ describe("fetchIssuesAndPullRequests — all repos monitored (edge case)", () => "octocat", undefined, undefined, - [{ fullName: "org/repo1" }] // all repos monitored + [{ owner: "org", name: "repo1", fullName: "org/repo1" }] // all repos monitored ); // Items returned from unfiltered search @@ -1754,7 +1754,7 @@ describe("fetchIssuesAndPullRequests — cross-feature: monitored repo + bot tra "octocat", undefined, [botUser], - [{ fullName: "org/monitored" }] + [{ owner: "org", name: "monitored", fullName: "org/monitored" }] ); // Both issues are present (no dedup since different IDs) @@ -1856,7 +1856,7 @@ describe("fetchIssuesAndPullRequests — unfiltered search error handling", () = "octocat", undefined, undefined, - [{ fullName: "org/monitored" }] + [{ owner: "org", name: "monitored", fullName: "org/monitored" }] ); // Partial issue data recovered @@ -1894,14 +1894,15 @@ describe("fetchIssuesAndPullRequests — unfiltered search error handling", () = "octocat", undefined, undefined, - [{ fullName: "org/monitored" }] + [{ owner: "org", name: "monitored", fullName: "org/monitored" }] ); // No results recovered expect(result.issues).toHaveLength(0); expect(result.pullRequests).toHaveLength(0); - // Error recorded + // Error recorded with retryable flag (network error → statusCode null → retryable) expect(result.errors.length).toBeGreaterThan(0); + expect(result.errors[0]).toMatchObject({ retryable: true }); }); }); @@ -1934,7 +1935,7 @@ describe("fetchIssuesAndPullRequests — all monitored + tracked users skips inv "octocat", undefined, [botUser], - [{ fullName: "org/repo1" }] // all repos monitored + [{ owner: "org", name: "repo1", fullName: "org/repo1" }] // all repos monitored ); // No involves: queries for tracked user (since normalRepos is empty, tracked search is skipped) @@ -1980,7 +1981,7 @@ describe("fetchIssuesAndPullRequests — onLightData suppression when all monito "octocat", onLightData, undefined, - [{ fullName: "org/repo1" }] // all repos monitored + [{ owner: "org", name: "repo1", fullName: "org/repo1" }] // all repos monitored ); // onLightData NOT called (main user search skipped, unfiltered results come after) From 77fee34e9530ff1be6c0c7ba4b4d3b2f378e3550 Mon Sep 17 00:00:00 2001 From: testvalue Date: Tue, 31 Mar 2026 08:20:31 -0400 Subject: [PATCH 06/10] fix(api): pins toLocaleString locale to en-US in notification strings --- src/app/services/api.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/app/services/api.ts b/src/app/services/api.ts index ca370fe5..8cb113c0 100644 --- a/src/app/services/api.ts +++ b/src/app/services/api.ts @@ -1103,13 +1103,13 @@ async function graphqlLightCombinedSearch( if (issues.length >= SEARCH_RESULT_CAP) { console.warn(`[api] Issue search results capped at ${SEARCH_RESULT_CAP}`); - pushNotification("search/issues", `Issue search results capped at ${SEARCH_RESULT_CAP.toLocaleString()} — some items are hidden`, "warning"); + pushNotification("search/issues", `Issue search results capped at ${SEARCH_RESULT_CAP.toLocaleString("en-US")} — some items are hidden`, "warning"); issues.splice(SEARCH_RESULT_CAP); } if (prMap.size >= SEARCH_RESULT_CAP) { console.warn(`[api] PR search results capped at ${SEARCH_RESULT_CAP}`); - pushNotification("search/prs", `PR search results capped at ${SEARCH_RESULT_CAP.toLocaleString()} — some items are hidden`, "warning"); + pushNotification("search/prs", `PR search results capped at ${SEARCH_RESULT_CAP.toLocaleString("en-US")} — some items are hidden`, "warning"); } const pullRequests = [...prMap.values()]; @@ -1209,13 +1209,13 @@ async function graphqlUnfilteredSearch( if (issues.length >= SEARCH_RESULT_CAP) { console.warn(`[api] Unfiltered issue results capped at ${SEARCH_RESULT_CAP}`); - pushNotification("search/unfiltered-issues", `Monitored repo issue results capped at ${SEARCH_RESULT_CAP.toLocaleString()} — some items are hidden`, "warning"); + pushNotification("search/unfiltered-issues", `Monitored repo issue results capped at ${SEARCH_RESULT_CAP.toLocaleString("en-US")} — some items are hidden`, "warning"); issues.splice(SEARCH_RESULT_CAP); } if (prMap.size >= SEARCH_RESULT_CAP) { console.warn(`[api] Unfiltered PR results capped at ${SEARCH_RESULT_CAP}`); - pushNotification("search/unfiltered-prs", `Monitored repo PR results capped at ${SEARCH_RESULT_CAP.toLocaleString()} — some items are hidden`, "warning"); + pushNotification("search/unfiltered-prs", `Monitored repo PR results capped at ${SEARCH_RESULT_CAP.toLocaleString("en-US")} — some items are hidden`, "warning"); } const pullRequests = [...prMap.values()]; @@ -1617,7 +1617,7 @@ async function graphqlSearchIssues( if (issues.length >= SEARCH_RESULT_CAP) { console.warn(`[api] Issue search results capped at ${SEARCH_RESULT_CAP}`); - pushNotification("search/issues", `Issue search results capped at ${SEARCH_RESULT_CAP.toLocaleString()} — some items are hidden`, "warning"); + pushNotification("search/issues", `Issue search results capped at ${SEARCH_RESULT_CAP.toLocaleString("en-US")} — some items are hidden`, "warning"); issues.splice(SEARCH_RESULT_CAP); } @@ -1757,7 +1757,7 @@ async function graphqlSearchPRs( if (prMap.size >= SEARCH_RESULT_CAP) { console.warn(`[api] PR search results capped at ${SEARCH_RESULT_CAP}`); - pushNotification("search/prs", `PR search results capped at ${SEARCH_RESULT_CAP.toLocaleString()} — some items are hidden`, "warning"); + pushNotification("search/prs", `PR search results capped at ${SEARCH_RESULT_CAP.toLocaleString("en-US")} — some items are hidden`, "warning"); } // Fork PR fallback: for PRs with null checkStatus where head repo owner differs from base From f8a0e8c6b2d6fa794135e1230d11b0472cf6f816 Mon Sep 17 00:00:00 2001 From: testvalue Date: Tue, 31 Mar 2026 08:31:32 -0400 Subject: [PATCH 07/10] fix: addresses remaining deferred findings (qa-5, perf-2) - adds poll notification reset reactive effect tests (qa-5): verifies _resetNotificationState fires on monitoredRepos and trackedUsers config changes, skips on initial mount, detects key swaps - adds .max(10) constraint on monitoredRepos schema (perf-2): limits unfiltered search volume, adds guard in setMonitoredRepo - adds schema constraint tests for monitoredRepos max --- src/app/stores/config.ts | 10 +- .../poll-notification-effects.test.ts | 114 ++++++++++++++++++ tests/stores/config.test.ts | 37 ++++++ 3 files changed, 159 insertions(+), 2 deletions(-) create mode 100644 tests/services/poll-notification-effects.test.ts diff --git a/src/app/stores/config.ts b/src/app/stores/config.ts index 9b3d86de..70bda6a4 100644 --- a/src/app/stores/config.ts +++ b/src/app/stores/config.ts @@ -1,6 +1,7 @@ import { z } from "zod"; import { createStore, produce } from "solid-js/store"; import { createEffect } from "solid-js"; +import { pushNotification } from "../lib/errors"; export const CONFIG_STORAGE_KEY = "github-tracker:config"; @@ -39,7 +40,7 @@ export const ConfigSchema = z.object({ selectedOrgs: z.array(z.string()).default([]), selectedRepos: z.array(RepoRefSchema).default([]), upstreamRepos: z.array(RepoRefSchema).default([]), - monitoredRepos: z.array(RepoRefSchema).default([]), + monitoredRepos: z.array(RepoRefSchema).max(10).default([]), trackedUsers: z.array(TrackedUserSchema).max(10).default([]), refreshInterval: z.number().min(0).max(3600).default(300), hotPollInterval: z.number().min(10).max(120).default(30), @@ -110,6 +111,7 @@ export function setMonitoredRepo(repo: z.infer, monitored: if (monitored) { const inSelected = draft.selectedRepos.some((r) => r.fullName === repo.fullName); if (!inSelected) return; + if (draft.monitoredRepos.length >= 10) return; const alreadyMonitored = draft.monitoredRepos.some((r) => r.fullName === repo.fullName); if (!alreadyMonitored) { draft.monitoredRepos.push(repo); @@ -132,7 +134,11 @@ export function initConfigPersistence(): void { const snapshot = JSON.parse(JSON.stringify(config)) as Config; clearTimeout(debounceTimer); debounceTimer = setTimeout(() => { - localStorage.setItem(CONFIG_STORAGE_KEY, JSON.stringify(snapshot)); + try { + localStorage.setItem(CONFIG_STORAGE_KEY, JSON.stringify(snapshot)); + } catch { + pushNotification("localStorage:config", "Config write failed — storage may be full", "warning"); + } }, 200); }); } diff --git a/tests/services/poll-notification-effects.test.ts b/tests/services/poll-notification-effects.test.ts new file mode 100644 index 00000000..57864acc --- /dev/null +++ b/tests/services/poll-notification-effects.test.ts @@ -0,0 +1,114 @@ +/** + * 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 "../../src/app/services/poll"; + +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("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(); + }); +}); diff --git a/tests/stores/config.test.ts b/tests/stores/config.test.ts index 62ce5440..d1e212b0 100644 --- a/tests/stores/config.test.ts +++ b/tests/stores/config.test.ts @@ -565,4 +565,41 @@ describe("setMonitoredRepo (C3)", () => { 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); + }); }); From b70601c334366d49315fac3771a458b7d1f8932a Mon Sep 17 00:00:00 2001 From: testvalue Date: Tue, 31 Mar 2026 09:57:11 -0400 Subject: [PATCH 08/10] fix(auth): adds localStorage resilience and 401 retry --- .../components/dashboard/DashboardPage.tsx | 11 +- .../onboarding/OnboardingWizard.tsx | 7 +- src/app/stores/auth.ts | 87 +++++++++--- src/app/stores/view.ts | 3 +- tests/components/DashboardPage.test.tsx | 11 +- tests/stores/auth.test.ts | 134 +++++++++++++++++- 6 files changed, 219 insertions(+), 34 deletions(-) diff --git a/src/app/components/dashboard/DashboardPage.tsx b/src/app/components/dashboard/DashboardPage.tsx index f35d31dc..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); diff --git a/src/app/components/onboarding/OnboardingWizard.tsx b/src/app/components/onboarding/OnboardingWizard.tsx index 32958250..19e52110 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"; @@ -67,7 +68,11 @@ export default function OnboardingWizard() { 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"); } diff --git a/src/app/stores/auth.ts b/src/app/stores/auth.ts index a0400a7e..98b1be7d 100644 --- a/src/app/stores/auth.ts +++ b/src/app/stores/auth.ts @@ -2,6 +2,7 @@ import { createSignal } from "solid-js"; import { clearCache } from "./cache"; import { CONFIG_STORAGE_KEY, resetConfig, updateConfig, config } from "./config"; import { VIEW_STORAGE_KEY, resetViewState } from "./view"; +import { pushNotification } from "../lib/errors"; export const AUTH_STORAGE_KEY = "github-tracker:auth-token"; export const DASHBOARD_STORAGE_KEY = "github-tracker:dashboard"; @@ -40,9 +41,13 @@ export { user }; // ── Actions ───────────────────────────────────────────────────────────────── export function setAuth(response: TokenExchangeResponse): void { - localStorage.setItem(AUTH_STORAGE_KEY, response.access_token); + try { + localStorage.setItem(AUTH_STORAGE_KEY, response.access_token); + } catch { + pushNotification("localStorage:auth", "Auth token write failed — storage may be full. Token exists in memory only this session.", "warning"); + } _setToken(response.access_token); - console.info("[auth] access token set (localStorage)"); + console.info("[auth] access token set"); } export function setAuthFromPat(token: string, userData: GitHubUser): void { @@ -89,37 +94,77 @@ export function clearAuth(): void { } } +/** Clear only the auth token. Preserves all user data (config, view state, dashboard + * cache) so the same user's preferences and cached data survive re-authentication. + * Used when a token becomes invalid (expired PAT, revoked OAuth) — NOT for explicit + * logout. Full data wipe (cross-user data isolation) is handled by clearAuth() + * which is reserved for explicit user actions (Sign out, Reset all). + * + * Callers MUST navigate away after calling this (e.g., window.location.replace or + * router navigate to /login). The poll coordinator is not stopped here — page + * navigation handles teardown. Use clearAuth() if not navigating. */ +export function expireToken(): void { + localStorage.removeItem(AUTH_STORAGE_KEY); + _setToken(null); + setUser(null); + console.info("[auth] token expired (user data preserved)"); +} + +const VALIDATE_HEADERS = { + Accept: "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", +} as const; + export async function validateToken(): Promise { const currentToken = _token(); if (!currentToken) return false; + const headers = { ...VALIDATE_HEADERS, Authorization: `Bearer ${currentToken}` }; + + function handleSuccess(userData: GitHubUser): true { + setUser({ login: userData.login, avatar_url: userData.avatar_url, name: userData.name }); + navigator.storage?.persist?.()?.catch(() => {}); + return true; + } + try { - const resp = await fetch("https://api.github.com/user", { - headers: { - Authorization: `Bearer ${currentToken}`, - Accept: "application/vnd.github+json", - "X-GitHub-Api-Version": "2022-11-28", - }, - }); + const resp = await fetch("https://api.github.com/user", { headers }); if (resp.ok) { - const userData = (await resp.json()) as GitHubUser; - setUser({ - login: userData.login, - avatar_url: userData.avatar_url, - name: userData.name, - }); - return true; + return handleSuccess((await resp.json()) as GitHubUser); } if (resp.status === 401) { - const method = config.authMethod; + // GitHub API can return transient 401s due to database replication lag. + // Retry once after a delay before invalidating the token. + await new Promise((r) => setTimeout(r, 1000)); + try { + const retry = await fetch("https://api.github.com/user", { headers }); + if (retry.ok) { + return handleSuccess((await retry.json()) as GitHubUser); + } + if (retry.status !== 401) { + // Non-auth error on retry (e.g. 500) — preserve token, try next load + return false; + } + } catch { + // Network error on retry — preserve token, try next load + return false; + } + + // Guard: if the token was replaced during the retry window (e.g., user + // re-authenticated via OAuth callback), don't invalidate the new token. + if (_token() !== currentToken) { + return false; + } + + // Both attempts returned 401 — token is genuinely invalid console.info( - method === "pat" - ? "[auth] PAT invalid or expired — clearing auth" - : "[auth] access token invalid — clearing auth" + config.authMethod === "pat" + ? "[auth] PAT invalid or expired — clearing token" + : "[auth] access token invalid — clearing token" ); - clearAuth(); + expireToken(); return false; } diff --git a/src/app/stores/view.ts b/src/app/stores/view.ts index dd65ae76..26f9ac7e 100644 --- a/src/app/stores/view.ts +++ b/src/app/stores/view.ts @@ -1,6 +1,7 @@ import { z } from "zod"; import { createStore, produce } from "solid-js/store"; import { createEffect, onCleanup, untrack } from "solid-js"; +import { pushNotification } from "../lib/errors"; export const VIEW_STORAGE_KEY = "github-tracker:view"; @@ -332,7 +333,7 @@ export function initViewPersistence(): void { try { localStorage.setItem(VIEW_STORAGE_KEY, json); } catch { - // QuotaExceededError — silently fail rather than kill the reactive graph + pushNotification("localStorage:view", "View state write failed — storage may be full", "warning"); } }, 200); onCleanup(() => { diff --git a/tests/components/DashboardPage.test.tsx b/tests/components/DashboardPage.test.tsx index 08a94ff9..987bddd0 100644 --- a/tests/components/DashboardPage.test.tsx +++ b/tests/components/DashboardPage.test.tsx @@ -26,6 +26,7 @@ vi.mock("@solidjs/router", () => ({ const authClearCallbacks: (() => void)[] = []; vi.mock("../../src/app/stores/auth", () => ({ clearAuth: vi.fn(), + expireToken: vi.fn(), token: () => "fake-token", user: () => ({ login: "testuser", avatar_url: "", name: "Test User" }), isAuthenticated: () => true, @@ -127,6 +128,7 @@ beforeEach(async () => { capturedFetchAll = null; capturedOnHotData = null; vi.mocked(authStore.clearAuth).mockClear(); + vi.mocked(authStore.expireToken).mockClear(); vi.mocked(pollService.fetchAllData).mockResolvedValue({ issues: [], pullRequests: [], @@ -296,18 +298,20 @@ describe("DashboardPage — auth error handling", () => { consoleErrorSpy.mockRestore(); }); - it("calls clearAuth and redirects to /login on 401 error (permanent token revoked)", async () => { + it("calls expireToken (not clearAuth) and redirects to /login on 401 error", async () => { const err401 = Object.assign(new Error("Unauthorized"), { status: 401 }); vi.mocked(pollService.fetchAllData).mockRejectedValue(err401); render(() => ); await waitFor(() => { - expect(authStore.clearAuth).toHaveBeenCalledOnce(); + expect(authStore.expireToken).toHaveBeenCalledOnce(); expect(mockLocationReplace).toHaveBeenCalledWith("/login"); }); + // clearAuth should NOT be called — user config/view preserved on token failure + expect(authStore.clearAuth).not.toHaveBeenCalled(); }); - it("does not call clearAuth for non-401 errors", async () => { + it("does not call expireToken or clearAuth for non-401 errors", async () => { const err500 = Object.assign(new Error("Server Error"), { status: 500 }); vi.mocked(pollService.fetchAllData).mockRejectedValue(err500); @@ -315,6 +319,7 @@ describe("DashboardPage — auth error handling", () => { // Flush all pending microtasks so the rejected promise settles await Promise.resolve(); await Promise.resolve(); + expect(authStore.expireToken).not.toHaveBeenCalled(); expect(authStore.clearAuth).not.toHaveBeenCalled(); expect(mockLocationReplace).not.toHaveBeenCalledWith("/login"); }); diff --git a/tests/stores/auth.test.ts b/tests/stores/auth.test.ts index 41ccb265..846fe308 100644 --- a/tests/stores/auth.test.ts +++ b/tests/stores/auth.test.ts @@ -131,6 +131,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 +253,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 +285,90 @@ 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("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 () => { From 6e3547c06a0fe6a1d40edc29a971805882bcd6e9 Mon Sep 17 00:00:00 2001 From: testvalue Date: Tue, 31 Mar 2026 10:26:35 -0400 Subject: [PATCH 09/10] fix: addresses PR review findings from code review --- .../onboarding/OnboardingWizard.tsx | 5 +- src/app/services/api.ts | 77 ++++++++++--------- tests/services/api.test.ts | 67 +++++++++++++++- .../poll-notification-effects.test.ts | 12 +++ tests/stores/auth.test.ts | 43 +++++++++++ 5 files changed, 161 insertions(+), 43 deletions(-) diff --git a/src/app/components/onboarding/OnboardingWizard.tsx b/src/app/components/onboarding/OnboardingWizard.tsx index 19e52110..480f9dc2 100644 --- a/src/app/components/onboarding/OnboardingWizard.tsx +++ b/src/app/components/onboarding/OnboardingWizard.tsx @@ -57,14 +57,11 @@ export default function OnboardingWizard() { function handleFinish() { const uniqueOrgs = [...new Set(selectedRepos().map((r) => r.owner))]; - // Prune monitoredRepos to only repos still in selectedRepos - const selectedSet = new Set(selectedRepos().map((r) => r.fullName)); - const prunedMonitoredRepos = monitoredRepos().filter((r) => selectedSet.has(r.fullName)); updateConfig({ selectedOrgs: uniqueOrgs, selectedRepos: selectedRepos(), upstreamRepos: upstreamRepos(), - monitoredRepos: prunedMonitoredRepos, + monitoredRepos: monitoredRepos(), onboardingComplete: true, }); // Flush synchronously — the debounced persistence effect won't fire before page unload diff --git a/src/app/services/api.ts b/src/app/services/api.ts index 8cb113c0..a0726018 100644 --- a/src/app/services/api.ts +++ b/src/app/services/api.ts @@ -914,8 +914,6 @@ export interface FetchIssuesAndPRsResult { interface LightSearchResult { issues: Issue[]; pullRequests: PullRequest[]; - /** GraphQL node IDs for phase 2 backfill */ - prNodeIds: string[]; errors: ApiError[]; } @@ -1058,6 +1056,32 @@ async function executeLightCombinedQuery( } } +/** Cap-check, notify, and assemble a LightSearchResult from working collections. */ +function finalizeSearchResult( + issues: Issue[], + prMap: Map, + errors: ApiError[], + issueSource: string, + prSource: string, + issueLabel: string, + prLabel: string, +): LightSearchResult { + if (issues.length >= SEARCH_RESULT_CAP) { + console.warn(`[api] ${issueLabel} capped at ${SEARCH_RESULT_CAP}`); + pushNotification(issueSource, `${issueLabel} capped at ${SEARCH_RESULT_CAP.toLocaleString("en-US")} — some items are hidden`, "warning"); + issues.splice(SEARCH_RESULT_CAP); + } + + if (prMap.size >= SEARCH_RESULT_CAP) { + console.warn(`[api] ${prLabel} capped at ${SEARCH_RESULT_CAP}`); + pushNotification(prSource, `${prLabel} capped at ${SEARCH_RESULT_CAP.toLocaleString("en-US")} — some items are hidden`, "warning"); + } + + const pullRequests = [...prMap.values()]; + if (pullRequests.length >= SEARCH_RESULT_CAP) pullRequests.splice(SEARCH_RESULT_CAP); + return { issues, pullRequests, errors }; +} + /** * Phase 1: light combined search. Fetches issues fully and PRs with minimal fields. * Returns light PRs (enriched: false) and their GraphQL node IDs for phase 2 backfill. @@ -1071,7 +1095,6 @@ async function graphqlLightCombinedSearch( return { issues: [], pullRequests: [], - prNodeIds: [], errors: [{ repo: "search", statusCode: null, message: "Invalid userLogin", retryable: false }], }; } @@ -1101,22 +1124,11 @@ async function graphqlLightCombinedSearch( } })); - if (issues.length >= SEARCH_RESULT_CAP) { - console.warn(`[api] Issue search results capped at ${SEARCH_RESULT_CAP}`); - pushNotification("search/issues", `Issue search results capped at ${SEARCH_RESULT_CAP.toLocaleString("en-US")} — some items are hidden`, "warning"); - issues.splice(SEARCH_RESULT_CAP); - } - - if (prMap.size >= SEARCH_RESULT_CAP) { - console.warn(`[api] PR search results capped at ${SEARCH_RESULT_CAP}`); - pushNotification("search/prs", `PR search results capped at ${SEARCH_RESULT_CAP.toLocaleString("en-US")} — some items are hidden`, "warning"); - } - - const pullRequests = [...prMap.values()]; - if (pullRequests.length >= SEARCH_RESULT_CAP) pullRequests.splice(SEARCH_RESULT_CAP); - const prNodeIds = pullRequests.map((pr) => nodeIdMap.get(pr.id)).filter((id): id is string => id != null); - - return { issues, pullRequests, prNodeIds, errors }; + return finalizeSearchResult( + issues, prMap, errors, + "search/issues", "search/prs", + "Issue search results", "PR search results", + ); } /** @@ -1129,7 +1141,7 @@ async function graphqlUnfilteredSearch( octokit: GitHubOctokit, repos: RepoRef[] ): Promise { - if (repos.length === 0) return { issues: [], pullRequests: [], prNodeIds: [], errors: [] }; + if (repos.length === 0) return { issues: [], pullRequests: [], errors: [] }; const chunks = chunkArray(repos, SEARCH_REPO_BATCH_SIZE); const issueSeen = new Set(); @@ -1207,21 +1219,11 @@ async function graphqlUnfilteredSearch( } })); - if (issues.length >= SEARCH_RESULT_CAP) { - console.warn(`[api] Unfiltered issue results capped at ${SEARCH_RESULT_CAP}`); - pushNotification("search/unfiltered-issues", `Monitored repo issue results capped at ${SEARCH_RESULT_CAP.toLocaleString("en-US")} — some items are hidden`, "warning"); - issues.splice(SEARCH_RESULT_CAP); - } - - if (prMap.size >= SEARCH_RESULT_CAP) { - console.warn(`[api] Unfiltered PR results capped at ${SEARCH_RESULT_CAP}`); - pushNotification("search/unfiltered-prs", `Monitored repo PR results capped at ${SEARCH_RESULT_CAP.toLocaleString("en-US")} — some items are hidden`, "warning"); - } - - const pullRequests = [...prMap.values()]; - if (pullRequests.length >= SEARCH_RESULT_CAP) pullRequests.splice(SEARCH_RESULT_CAP); - const prNodeIds = pullRequests.map((pr) => nodeIdMap.get(pr.id)).filter((id): id is string => id != null); - return { issues, pullRequests, prNodeIds, errors }; + return finalizeSearchResult( + issues, prMap, errors, + "search/unfiltered-issues", "search/unfiltered-prs", + "Monitored repo issue results", "Monitored repo PR results", + ); } // ── Two-phase: heavy backfill ───────────────────────────────────────────────── @@ -1356,7 +1358,6 @@ function mergeEnrichment( * Merges tracked user search results into the main issue/PR maps. * Items already present get the tracked user's login appended to surfacedBy. * New items are added with surfacedBy: [trackedLogin]. - * Returns a union of all prNodeIds for backfill. */ function mergeTrackedUserResults( issueMap: Map, @@ -1475,7 +1476,7 @@ export async function fetchIssuesAndPullRequests( // Unfiltered search for monitored repos — runs in parallel with tracked searches const unfilteredPromise = monitoredReposList.length > 0 ? graphqlUnfilteredSearch(octokit, monitoredReposList) - : Promise.resolve({ issues: [], pullRequests: [], prNodeIds: [], errors: [] } as LightSearchResult); + : Promise.resolve({ issues: [], pullRequests: [], errors: [] } as LightSearchResult); const [mainBackfill, trackedResults, unfilteredResult] = await Promise.all([ mainBackfillPromise, @@ -2210,7 +2211,7 @@ export async function validateGitHubUser( login: raw.login.toLowerCase(), avatarUrl, name: raw.name ?? null, - type: raw.type === "Bot" ? "bot" as const : "user" as const, + type: raw.type === "Bot" ? "bot" : "user", }; } diff --git a/tests/services/api.test.ts b/tests/services/api.test.ts index 16bc1450..0eb6de64 100644 --- a/tests/services/api.test.ts +++ b/tests/services/api.test.ts @@ -475,7 +475,7 @@ describe("fetchIssues", () => { const result = await fetchIssues( octokit as never, [testRepo], - "bad user" // contains space — fails VALID_LOGIN + "bad user" // contains space — fails VALID_TRACKED_LOGIN ); expect(result.issues).toEqual([]); @@ -1904,6 +1904,71 @@ describe("fetchIssuesAndPullRequests — unfiltered search error handling", () = 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", () => { diff --git a/tests/services/poll-notification-effects.test.ts b/tests/services/poll-notification-effects.test.ts index 57864acc..e5a85533 100644 --- a/tests/services/poll-notification-effects.test.ts +++ b/tests/services/poll-notification-effects.test.ts @@ -95,6 +95,18 @@ describe("poll.ts — notification reset reactive effects", () => { 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: [ diff --git a/tests/stores/auth.test.ts b/tests/stores/auth.test.ts index 846fe308..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", () => { @@ -351,6 +374,26 @@ describe("validateToken", () => { 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({}) }) From cb87358c0f1d595493d85f5aafe9d39ce5a2bec1 Mon Sep 17 00:00:00 2001 From: testvalue Date: Tue, 31 Mar 2026 12:26:32 -0400 Subject: [PATCH 10/10] fix: monitor-all gate bypass, hot poll guard, and UX - invalidates notifications gate when monitoredRepos or trackedUsers change so next poll fetches fresh data instead of returning stale 304 - tightens hot poll PR qualifier to require enriched === true, excluding unenriched and null-checkStatus PRs from unnecessary polling - switches monitor-all eyeball icon from opacity toggle to text-info color for better enabled/disabled contrast - adds "Monitoring all" indicator with repo names on Settings page - adds regex constraints to RepoRefSchema for defense-in-depth validation - sets makePullRequest helper default to enriched: true (steady state) - adds gate bypass integration tests and Settings indicator render tests --- .../components/onboarding/RepoSelector.tsx | 5 +- src/app/components/settings/SettingsPage.tsx | 15 +++- src/app/services/poll.ts | 4 +- src/app/stores/config.ts | 8 +- .../components/settings/SettingsPage.test.tsx | 25 +++++++ tests/helpers/index.tsx | 1 + tests/services/hot-poll.test.ts | 42 +++++------ .../poll-notification-effects.test.ts | 73 ++++++++++++++++++- 8 files changed, 145 insertions(+), 28 deletions(-) diff --git a/src/app/components/onboarding/RepoSelector.tsx b/src/app/components/onboarding/RepoSelector.tsx index a8741ba7..ab0e26de 100644 --- a/src/app/components/onboarding/RepoSelector.tsx +++ b/src/app/components/onboarding/RepoSelector.tsx @@ -563,7 +563,10 @@ export default function RepoSelector(props: RepoSelectorProps) { props.onMonitorToggle?.(toRepoRef(repo()), !monitoredSet().has(repo().fullName)); }} class="btn btn-ghost btn-sm btn-circle mr-2" - classList={{ "opacity-40": !monitoredSet().has(repo().fullName) }} + classList={{ + "text-info": monitoredSet().has(repo().fullName), + "text-base-content/20": !monitoredSet().has(repo().fullName), + }} title={monitoredSet().has(repo().fullName) ? "Stop monitoring all activity" : "Monitor all activity"} aria-label={monitoredSet().has(repo().fullName) ? "Stop monitoring all activity" : "Monitor all activity"} aria-pressed={monitoredSet().has(repo().fullName)} diff --git a/src/app/components/settings/SettingsPage.tsx b/src/app/components/settings/SettingsPage.tsx index 79576474..f3db7e9c 100644 --- a/src/app/components/settings/SettingsPage.tsx +++ b/src/app/components/settings/SettingsPage.tsx @@ -1,4 +1,4 @@ -import { createSignal, Show, onCleanup } from "solid-js"; +import { createSignal, createMemo, Show, onCleanup } from "solid-js"; import { useNavigate } from "@solidjs/router"; import { config, updateConfig, setMonitoredRepo } from "../../stores/config"; import { clearAuth } from "../../stores/auth"; @@ -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() { @@ -305,6 +309,15 @@ export default function SettingsPage() { Tracking {localRepos().length} repos will use significant API quota per poll cycle

    + 0}> +

    + + Monitoring all: {monitoredRepoNames()} +

    +