From 13634e57df704b2892e1b6cacd74e1ccea6f19d8 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Thu, 6 Aug 2026 11:03:40 -0400 Subject: [PATCH] fix(config): reconcile stale scope references and paginate org fetch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit selectedOrgs could retain logins no longer returned by fetchOrgs() (revoked/removed org access), inflating the "N of M selected" counter past its live total. fetchOrgs() itself only fetched a single page, so fixing that counter safely required paginating it first — done via octokit.paginate.iterator, matching fetchRepos(). OrgSelector now prunes stale selectedOrgs entries once a full fetch resolves and notifies the user. customTabs[].orgScope/repoScope had the same class of bug: nothing reconciled them when selectedRepos shrank, unlike monitoredRepos which already did. updateConfig() now prunes both scope arrays the same way. Since scope pruning can empty a tab's scope entirely, and empty scope previously meant "match all repos" (a silent, surprising expansion), empty scope now means "match nothing" instead, with a warning icon on the tab (TabBar and Settings) so it's never a silent state. Added a structural regression test enumerating every config field that derives from selectedRepos, so a future field added without reconciliation fails the suite by default. --- .../components/dashboard/DashboardPage.tsx | 12 +- src/app/components/layout/TabBar.tsx | 9 +- src/app/components/onboarding/OrgSelector.tsx | 23 ++- .../components/settings/CustomTabsSection.tsx | 15 +- src/app/components/shared/CustomTabModal.tsx | 11 +- src/app/lib/format.ts | 2 +- src/app/services/api.ts | 26 +-- src/app/stores/config.ts | 15 +- src/shared/schemas.ts | 5 + tests/components/DashboardPage.test.tsx | 47 ++++- tests/components/layout/TabBar.test.tsx | 24 +++ .../onboarding/OrgSelector.test.tsx | 73 +++++++ .../settings/CustomTabsSection.test.tsx | 12 +- tests/lib/format.test.ts | 4 +- tests/services/api.test.ts | 31 ++- tests/stores/config.test.ts | 187 ++++++++++++++++++ 16 files changed, 460 insertions(+), 36 deletions(-) diff --git a/src/app/components/dashboard/DashboardPage.tsx b/src/app/components/dashboard/DashboardPage.tsx index 8356bb86..6b96ad1b 100644 --- a/src/app/components/dashboard/DashboardPage.tsx +++ b/src/app/components/dashboard/DashboardPage.tsx @@ -8,7 +8,7 @@ import IssuesTab from "./IssuesTab"; import PullRequestsTab from "./PullRequestsTab"; import TrackedTab from "./TrackedTab"; import PersonalSummaryStrip from "./PersonalSummaryStrip"; -import { config, setConfig, getCustomTab, isBuiltinTab, isActionsBasedTab, updateJiraConfig, type TrackedUser } from "../../stores/config"; +import { config, setConfig, getCustomTab, isBuiltinTab, isActionsBasedTab, isTabUnscoped, updateJiraConfig, type TrackedUser } from "../../stores/config"; import { viewState, updateViewState, setSortPreference, pruneClosedTrackedItems, removeCustomTabState, untrackJiraItem, setTabFilter, IssueFiltersSchema, PullRequestFiltersSchema, ActionsFiltersSchema } from "../../stores/view"; import DependenciesTab from "./DependenciesTab"; import { isDependencyPr, expandBotLogins, needsBodyFallback, parseRenovateBody, type VersionInfo } from "../../lib/dependency-detection"; @@ -54,14 +54,18 @@ const ISSUE_FILTER_DEFAULTS = IssueFiltersSchema.parse({}); const PR_FILTER_DEFAULTS = PullRequestFiltersSchema.parse({}); const ACTIONS_FILTER_DEFAULTS = ActionsFiltersSchema.parse({}); -/** Build a scope matcher for a custom tab's org/repo scope. Shared between customTabData and tabCounts. */ +/** + * Build a scope matcher for a custom tab's org/repo scope. Shared between + * customTabData and tabCounts. An empty scope (no orgs, no repos) matches + * nothing — see isTabUnscoped, which flags this state for the UI. + */ function buildTabScopeMatcher(tab: CustomTab): (repoFullName: string) => boolean { const orgSet = tab.orgScope.length > 0 ? new Set(tab.orgScope.map((o) => o.toLowerCase())) : null; const repoSet = tab.repoScope.length > 0 ? new Set(tab.repoScope.map((r) => r.fullName.toLowerCase())) : null; return (repoFullName: string) => { if (repoSet && repoSet.has(repoFullName.toLowerCase())) return true; if (orgSet && orgSet.has(repoFullName.split("/")[0].toLowerCase())) return true; - return !orgSet && !repoSet; + return false; }; } @@ -1270,7 +1274,7 @@ export default function DashboardPage() { enableActions={config.enableActions} enableJira={!!config.jira?.enabled} enableDependencies={enableDependencies()} - customTabs={config.customTabs.filter((t) => config.enableActions || t.baseType !== "actions").map((t) => ({ id: t.id, name: t.name }))} + customTabs={config.customTabs.filter((t) => config.enableActions || t.baseType !== "actions").map((t) => ({ id: t.id, name: t.name, isUnscoped: isTabUnscoped(t) }))} onAddTab={() => setShowCustomTabModal(true)} onEditTab={(id) => { setEditingTabId(id); setShowCustomTabModal(true); }} /> diff --git a/src/app/components/layout/TabBar.tsx b/src/app/components/layout/TabBar.tsx index 8fa29906..72dd378a 100644 --- a/src/app/components/layout/TabBar.tsx +++ b/src/app/components/layout/TabBar.tsx @@ -14,7 +14,7 @@ interface TabBarProps { enableActions?: boolean; enableJira?: boolean; enableDependencies?: boolean; - customTabs?: Array<{ id: string; name: string }>; + customTabs?: Array<{ id: string; name: string; isUnscoped?: boolean }>; onAddTab?: () => void; onEditTab?: (id: string) => void; } @@ -77,6 +77,13 @@ export default function TabBar(props: TabBarProps) { {(tab) => (
+ + + + + + + {tab.name} {props.counts?.[tab.id]} diff --git a/src/app/components/onboarding/OrgSelector.tsx b/src/app/components/onboarding/OrgSelector.tsx index 8f438486..9d8850f4 100644 --- a/src/app/components/onboarding/OrgSelector.tsx +++ b/src/app/components/onboarding/OrgSelector.tsx @@ -1,6 +1,7 @@ -import { createSignal, createResource, For, Show } from "solid-js"; +import { createSignal, createResource, createEffect, untrack, For, Show } from "solid-js"; import { fetchOrgs, OrgEntry } from "../../services/api"; import { getClient } from "../../services/github"; +import { pushNotification } from "../../lib/errors"; import LoadingSpinner from "../shared/LoadingSpinner"; import FilterInput from "../shared/FilterInput"; @@ -25,6 +26,26 @@ export default function OrgSelector(props: OrgSelectorProps) { return all.filter((o) => o.login.toLowerCase().includes(q)); }; + // Prune selectedOrgs entries no longer present in a fresh, fully-paginated + // fetchOrgs() result — e.g. revoked/removed org access. Without this, a + // stale login would inflate the "N selected" counters below past the live + // total (selected.length could exceed orgs().length). + createEffect(() => { + const list = orgs(); + if (orgs.loading || orgs.error || !list) return; + const liveLogins = new Set(list.map((o) => o.login.toLowerCase())); + const current = untrack(() => props.selected); + const stale = current.filter((login) => !liveLogins.has(login.toLowerCase())); + if (stale.length === 0) return; + const pruned = current.filter((login) => liveLogins.has(login.toLowerCase())); + untrack(() => props.onChange(pruned)); + pushNotification( + "org-prune", + `Removed ${stale.length} organization${stale.length !== 1 ? "s" : ""} you no longer have access to (${stale.join(", ")})`, + "warning" + ); + }); + const isSelected = (login: string) => props.selected.includes(login); function toggleOrg(login: string) { diff --git a/src/app/components/settings/CustomTabsSection.tsx b/src/app/components/settings/CustomTabsSection.tsx index 82860d9c..4d3b5790 100644 --- a/src/app/components/settings/CustomTabsSection.tsx +++ b/src/app/components/settings/CustomTabsSection.tsx @@ -1,5 +1,5 @@ import { createSignal, createMemo, For, Show } from "solid-js"; -import { config, removeCustomTab, reorderCustomTab } from "../../stores/config"; +import { config, removeCustomTab, reorderCustomTab, isTabUnscoped } from "../../stores/config"; import type { CustomTab } from "../../stores/config"; import type { RepoRef } from "../../services/api"; import CustomTabModal from "../shared/CustomTabModal"; @@ -82,7 +82,18 @@ export default function CustomTabsSection(props: CustomTabsSectionProps) { {baseTypeLabel(tab.baseType)} - {formatScopeSummary(tab.orgScope.length, tab.repoScope.length, true)} + + + + + + + + + + {formatScopeSummary(tab.orgScope.length, tab.repoScope.length, true)} + + {tab.exclusive ? ( diff --git a/src/app/components/shared/CustomTabModal.tsx b/src/app/components/shared/CustomTabModal.tsx index 870106a4..b3e0e7c3 100644 --- a/src/app/components/shared/CustomTabModal.tsx +++ b/src/app/components/shared/CustomTabModal.tsx @@ -68,6 +68,8 @@ export default function CustomTabModal(props: CustomTabModalProps) { const nameValid = createMemo(() => name().trim().length > 0 && name().trim().length <= 30); + const scopeIsEmpty = createMemo(() => selectedOrgs().size === 0 && selectedRepos().size === 0); + // User field group — dynamic, includes tracked user logins const userFieldGroup = createMemo((): FilterChipGroupDef => ({ label: "User", @@ -258,7 +260,12 @@ export default function CustomTabModal(props: CustomTabModalProps) { onClick={() => setScopeOpen((v) => !v)} > Scope - + + + + + + {formatScopeSummary(selectedOrgs().size, selectedRepos().size)} {scopeOpen() ? "▲" : "▼"} @@ -266,7 +273,7 @@ export default function CustomTabModal(props: CustomTabModalProps) {

- Leave empty to include all repos. Org selection includes all repos in that org. + Leave empty to match no repos — the tab will show a warning icon until scoped. Org selection includes all repos in that org.

0} diff --git a/src/app/lib/format.ts b/src/app/lib/format.ts index fe48e008..04993943 100644 --- a/src/app/lib/format.ts +++ b/src/app/lib/format.ts @@ -22,7 +22,7 @@ export function rateLimitCssClass(remaining: number, limit: number): string { /** Format scope counts as "N org(s), M repo(s)". When elideZero is true, omit zero-count segments. */ export function formatScopeSummary(orgCount: number, repoCount: number, elideZero = false): string { - if (orgCount === 0 && repoCount === 0) return "All repos"; + if (orgCount === 0 && repoCount === 0) return "No repos selected"; if (elideZero) { const parts: string[] = []; if (orgCount > 0) parts.push(`${orgCount} org${orgCount !== 1 ? "s" : ""}`); diff --git a/src/app/services/api.ts b/src/app/services/api.ts index ece1bca2..bba40f31 100644 --- a/src/app/services/api.ts +++ b/src/app/services/api.ts @@ -1553,19 +1553,29 @@ function mapReviewDecision( /** * Returns orgs and the personal user account. Personal account is first. + * Fully paginates GET /user/orgs — a single-page fetch would silently + * truncate at 100 orgs for heavily-affiliated accounts. */ export async function fetchOrgs( octokit: ReturnType ): Promise { if (!octokit) throw new Error("No GitHub client available"); - const [userResult, orgsResult] = await Promise.all([ - cachedRequest(octokit, "orgs:user", "GET /user"), - cachedRequest(octokit, "orgs:all", "GET /user/orgs", { per_page: 100 }), - ]); + const userPromise = cachedRequest(octokit, "orgs:user", "GET /user"); + + const orgEntries: OrgEntry[] = []; + const ORG_CAP = 1000; + for await (const response of octokit.paginate.iterator("GET /user/orgs", { + per_page: 100, + })) { + for (const org of response.data as RawOrg[]) { + orgEntries.push({ login: org.login, avatarUrl: org.avatar_url, type: "org" }); + } + if (orgEntries.length >= ORG_CAP) break; + } + const userResult = await userPromise; const user = userResult.data as RawUser; - const orgs = orgsResult.data as RawOrg[]; const personal: OrgEntry = { login: user.login, @@ -1573,12 +1583,6 @@ export async function fetchOrgs( type: "user", }; - const orgEntries: OrgEntry[] = orgs.map((o) => ({ - login: o.login, - avatarUrl: o.avatar_url, - type: "org", - })); - return [personal, ...orgEntries]; } diff --git a/src/app/stores/config.ts b/src/app/stores/config.ts index 8fde1ee9..eb01885d 100644 --- a/src/app/stores/config.ts +++ b/src/app/stores/config.ts @@ -9,7 +9,7 @@ import { z } from "zod"; // ── Re-exports from shared/schemas (backward compat for existing importers) ─── export { ConfigSchema, RepoRefSchema, TrackedUserSchema, THEME_OPTIONS, - CustomTabSchema, BUILTIN_TAB_IDS, isBuiltinTab, isActionsBasedTab, + CustomTabSchema, BUILTIN_TAB_IDS, isBuiltinTab, isActionsBasedTab, isTabUnscoped, type Config, type TrackedUser, type ThemeId, type CustomTab, type BuiltinTabId, type JiraConfig, } from "../../shared/schemas"; @@ -82,6 +82,19 @@ export function updateConfig(partial: Partial): void { if ("selectedRepos" in partial) { const selectedSet = new Set(draft.selectedRepos.map((r) => r.fullName)); draft.monitoredRepos = draft.monitoredRepos.filter((r) => selectedSet.has(r.fullName)); + + // customTabs scope is derived from selectedRepos (see CustomTabModal's + // availableOrgs/availableRepos props) — prune scope entries that no + // longer correspond to a tracked repo, same as monitoredRepos above. + const ownerSet = new Set(draft.selectedRepos.map((r) => r.owner.toLowerCase())); + draft.customTabs = draft.customTabs.map((tab) => { + const filteredOrgScope = tab.orgScope.filter((o) => ownerSet.has(o.toLowerCase())); + const filteredRepoScope = tab.repoScope.filter((r) => selectedSet.has(r.fullName)); + if (filteredOrgScope.length === tab.orgScope.length && filteredRepoScope.length === tab.repoScope.length) { + return tab; + } + return { ...tab, orgScope: filteredOrgScope, repoScope: filteredRepoScope }; + }); } }) ); diff --git a/src/shared/schemas.ts b/src/shared/schemas.ts index 94aec5dc..0aa97d5c 100644 --- a/src/shared/schemas.ts +++ b/src/shared/schemas.ts @@ -54,6 +54,11 @@ export function isActionsBasedTab(id: string, customTabs: readonly CustomTab[]): return id === "actions" || (!isBuiltinTab(id) && customTabs.some((t) => t.id === id && t.baseType === "actions")); } +/** A tab with no orgScope and no repoScope matches no repos — see buildTabScopeMatcher in DashboardPage.tsx. */ +export function isTabUnscoped(tab: Pick): boolean { + return tab.orgScope.length === 0 && tab.repoScope.length === 0; +} + export const JiraAuthMethodSchema = z.enum(["oauth", "token"]).default("oauth"); export const JiraCustomFieldSchema = z.object({ diff --git a/tests/components/DashboardPage.test.tsx b/tests/components/DashboardPage.test.tsx index 7f687df2..d3775f74 100644 --- a/tests/components/DashboardPage.test.tsx +++ b/tests/components/DashboardPage.test.tsx @@ -1050,12 +1050,12 @@ describe("DashboardPage — tracked tab", () => { describe("DashboardPage — exclusive custom tabs", () => { it("exclusive issues tab removes claimed items from the builtin Issues badge", async () => { - // Add an exclusive issues custom tab that claims all repos + // Add an exclusive issues custom tab scoped to the fixture's default owner configStore.addCustomTab({ id: "excl01", name: "My Issues", baseType: "issues", - orgScope: [], + orgScope: ["owner"], repoScope: [], filterPreset: {}, exclusive: true, @@ -1083,7 +1083,7 @@ describe("DashboardPage — exclusive custom tabs", () => { id: "excl02", name: "Exclusive PRs", baseType: "pullRequests", - orgScope: [], + orgScope: ["owner"], repoScope: [], filterPreset: {}, exclusive: true, @@ -1144,7 +1144,7 @@ describe("DashboardPage — exclusive custom tabs", () => { id: "first01", name: "First Exclusive", baseType: "issues", - orgScope: [], + orgScope: ["owner"], repoScope: [], filterPreset: {}, exclusive: true, @@ -1153,7 +1153,7 @@ describe("DashboardPage — exclusive custom tabs", () => { id: "second01", name: "Second Exclusive", baseType: "issues", - orgScope: [], + orgScope: ["owner"], repoScope: [], filterPreset: {}, exclusive: true, @@ -1183,7 +1183,7 @@ describe("DashboardPage — exclusive custom tabs", () => { id: "exclact01", name: "My Actions", baseType: "actions", - orgScope: [], + orgScope: ["owner"], repoScope: [], filterPreset: {}, exclusive: true, @@ -1380,6 +1380,37 @@ describe("DashboardPage — custom tab scoping", () => { expect(customTab.textContent?.replace(/\D+/g, "")).toBe("1"); }); }); + + it("a tab with empty orgScope and repoScope matches no items and shows the unscoped icon", async () => { + configStore.addCustomTab({ + id: "unscoped01", + name: "Unscoped Tab", + baseType: "issues", + orgScope: [], + repoScope: [], + filterPreset: { scope: "all" }, + exclusive: false, + }); + vi.mocked(pollService.fetchAllData).mockResolvedValue({ + issues: [ + makeIssue({ id: 60, title: "Some issue", repoFullName: "owner/repo" }), + ], + pullRequests: [], + workflowRuns: [], + errors: [], + }); + + render(() => ); + await waitFor(() => { + // Empty scope now matches nothing — not "all repos" as it did previously. + const customTab = screen.getByRole("tab", { name: /Unscoped Tab/ }); + expect(customTab.textContent?.replace(/\D+/g, "")).toBe("0"); + // The builtin Issues tab is unaffected — it still shows the item. + const issuesTab = screen.getByRole("tab", { name: /^Issues/ }); + expect(issuesTab.textContent?.replace(/\D+/g, "")).toBe("1"); + expect(screen.getByLabelText("Unscoped tab")).toBeDefined(); + }); + }); }); // ── resolveInitialTab stale custom tab fallback ────────────────────────────── @@ -1535,7 +1566,7 @@ describe("DashboardPage — tabCounts applies filterPreset", () => { id: "selfuser", name: "My Items", baseType: "issues", - orgScope: [], + orgScope: ["owner"], repoScope: [], filterPreset: { scope: "all", user: "_self" }, exclusive: false, @@ -1567,7 +1598,7 @@ describe("DashboardPage — tabCounts applies filterPreset", () => { id: "failures", name: "Failed Runs", baseType: "actions", - orgScope: [], + orgScope: ["owner"], repoScope: [], filterPreset: { conclusion: "failure" }, exclusive: false, diff --git a/tests/components/layout/TabBar.test.tsx b/tests/components/layout/TabBar.test.tsx index 366b13a4..fcd001ef 100644 --- a/tests/components/layout/TabBar.test.tsx +++ b/tests/components/layout/TabBar.test.tsx @@ -176,6 +176,30 @@ describe("TabBar", () => { screen.getByText("7"); }); + it("shows a warning icon on an unscoped custom tab", () => { + const onTabChange = vi.fn(); + render(() => ( + + )); + screen.getByLabelText("Unscoped tab"); + }); + + it("does not show a warning icon on a scoped custom tab", () => { + const onTabChange = vi.fn(); + render(() => ( + + )); + expect(screen.queryByLabelText("Unscoped tab")).toBeNull(); + }); + it("does not render a count badge when count is undefined for a custom tab", () => { const onTabChange = vi.fn(); render(() => ( diff --git a/tests/components/onboarding/OrgSelector.test.tsx b/tests/components/onboarding/OrgSelector.test.tsx index bb65b2f8..b6a5d72e 100644 --- a/tests/components/onboarding/OrgSelector.test.tsx +++ b/tests/components/onboarding/OrgSelector.test.tsx @@ -1,4 +1,5 @@ import { describe, it, expect, vi, beforeEach, beforeAll, afterAll } from "vitest"; +import { createSignal } from "solid-js"; import { render, screen, fireEvent, waitFor } from "@solidjs/testing-library"; import userEvent from "@testing-library/user-event"; import type { OrgEntry } from "../../../src/app/services/api"; @@ -17,7 +18,12 @@ vi.mock("../../../src/app/services/api", async (importOriginal) => { }; }); +vi.mock("../../../src/app/lib/errors", () => ({ + pushNotification: vi.fn(), +})); + import * as api from "../../../src/app/services/api"; +import { pushNotification } from "../../../src/app/lib/errors"; import OrgSelector from "../../../src/app/components/onboarding/OrgSelector"; const mockOrgs: OrgEntry[] = [ @@ -182,4 +188,71 @@ describe("OrgSelector", () => { screen.getByText(/1 of 3 selected/i); }); }); + + // ── Stale selectedOrgs pruning (revoked/removed org access) ──────────────── + // Regression coverage for the "N of M selected" counter going out of range + // (selected.length > orgs().length) when config.selectedOrgs retains a login + // no longer returned by a fresh fetchOrgs() — e.g. revoked org access. + + describe("stale org pruning", () => { + it("prunes a selected login no longer present in the live fetch and notifies", async () => { + vi.mocked(api.fetchOrgs).mockResolvedValue(mockOrgs); + const onChange = vi.fn(); + render(() => ( + + )); + + await waitFor(() => { + expect(onChange).toHaveBeenCalledWith(["myorg"]); + }); + expect(pushNotification).toHaveBeenCalledWith( + "org-prune", + expect.stringContaining("revoked-org"), + "warning" + ); + }); + + it("is case-insensitive when matching selected logins against the live fetch", async () => { + vi.mocked(api.fetchOrgs).mockResolvedValue(mockOrgs); + const onChange = vi.fn(); + render(() => ); + + await waitFor(() => { + screen.getByText("myorg"); + }); + expect(onChange).not.toHaveBeenCalled(); + expect(pushNotification).not.toHaveBeenCalled(); + }); + + it("does not prune or notify when every selected login is still present", async () => { + vi.mocked(api.fetchOrgs).mockResolvedValue(mockOrgs); + const onChange = vi.fn(); + render(() => ( + + )); + + await waitFor(() => { + screen.getByText("myorg"); + }); + expect(onChange).not.toHaveBeenCalled(); + expect(pushNotification).not.toHaveBeenCalled(); + }); + + it("settles on a valid 'N of M' count once the parent applies the pruned selection", async () => { + // End-to-end reproduction of the "21 of 20 selected" bug: a parent that + // wires onChange back into its own selected state (as SettingsPage does + // via localOrgs/handleOrgsChange) must land on a numerator <= denominator. + vi.mocked(api.fetchOrgs).mockResolvedValue(mockOrgs); + + function Wrapper() { + const [selected, setSelected] = createSignal(["myorg", "anotheorg", "revoked-org"]); + return ; + } + render(() => ); + + await waitFor(() => { + screen.getByText(/2 of 3 selected/i); + }); + }); + }); }); diff --git a/tests/components/settings/CustomTabsSection.test.tsx b/tests/components/settings/CustomTabsSection.test.tsx index 3c6da861..f7cbf75c 100644 --- a/tests/components/settings/CustomTabsSection.test.tsx +++ b/tests/components/settings/CustomTabsSection.test.tsx @@ -38,6 +38,8 @@ vi.mock("../../../src/app/stores/config", () => ({ reorderCustomTab: mockReorderCustomTab, addCustomTab: mockAddCustomTab, updateCustomTab: mockUpdateCustomTab, + isTabUnscoped: (tab: { orgScope: string[]; repoScope: unknown[] }) => + tab.orgScope.length === 0 && tab.repoScope.length === 0, })); vi.mock("../../../src/app/stores/view", () => ({ @@ -148,9 +150,15 @@ describe("CustomTabsSection — table rendering", () => { expect(badges.length).toBeGreaterThanOrEqual(1); }); - it("shows 'All repos' when no scope is configured", () => { + it("shows 'No repos selected' and a warning icon when no scope is configured", () => { renderSection([makeTab({ orgScope: [], repoScope: [] })]); - expect(screen.getByText("All repos")).toBeDefined(); + expect(screen.getByText("No repos selected")).toBeDefined(); + expect(screen.getByLabelText("Unscoped tab")).toBeDefined(); + }); + + it("does not show the unscoped warning icon when scope is configured", () => { + renderSection([makeTab({ orgScope: ["myorg"], repoScope: [] })]); + expect(screen.queryByLabelText("Unscoped tab")).toBeNull(); }); it("shows org count summary in scope column", () => { diff --git a/tests/lib/format.test.ts b/tests/lib/format.test.ts index 07ea7893..42c251e4 100644 --- a/tests/lib/format.test.ts +++ b/tests/lib/format.test.ts @@ -378,8 +378,8 @@ describe("formatCount", () => { }); describe("formatScopeSummary", () => { - it("returns 'All repos' when both orgCount and repoCount are 0", () => { - expect(formatScopeSummary(0, 0)).toBe("All repos"); + it("returns 'No repos selected' when both orgCount and repoCount are 0", () => { + expect(formatScopeSummary(0, 0)).toBe("No repos selected"); }); it("elideZero=true with only orgCount returns '1 org'", () => { diff --git a/tests/services/api.test.ts b/tests/services/api.test.ts index 904eee72..cbdfd562 100644 --- a/tests/services/api.test.ts +++ b/tests/services/api.test.ts @@ -40,7 +40,9 @@ function makeOctokit( void params; // captured for test assertions // For tests that need paginate.iterator, return a single page const data = - route.includes("/orgs/") || route.includes("/user/repos") + route === "GET /user/orgs" + ? orgsFixture.filter((o) => o.type === "Organization") + : route.includes("/orgs/") || route.includes("/user/repos") ? reposFixture : []; return (async function* () { @@ -105,6 +107,33 @@ describe("fetchOrgs", () => { it("throws when octokit is null", async () => { await expect(fetchOrgs(null)).rejects.toThrow("No GitHub client available"); }); + + it("paginates past a single page of 100 orgs", async () => { + const page1 = Array.from({ length: 100 }, (_, i) => ({ + login: `org-${i}`, avatar_url: "https://avatars.githubusercontent.com/u/1", type: "Organization", + })); + const page2 = [{ login: "org-101", avatar_url: "https://avatars.githubusercontent.com/u/2", type: "Organization" }]; + const octokit = { + request: vi.fn(async (route: string) => { + if (route === "GET /user") { + return { data: { login: "octocat", avatar_url: "https://github.com/images/error/octocat_happy.gif" }, headers: {} }; + } + return { data: [], headers: {} }; + }), + graphql: vi.fn(), + paginate: { + iterator: vi.fn(() => (async function* () { + yield { data: page1 }; + yield { data: page2 }; + })()), + }, + }; + const result = await fetchOrgs(octokit as unknown as ReturnType); + + // Personal account + 101 orgs across two pages — none dropped. + expect(result.length).toBe(102); + expect(result.some((o) => o.login === "org-101")).toBe(true); + }); }); // ── fetchRepos ──────────────────────────────────────────────────────────────── diff --git a/tests/stores/config.test.ts b/tests/stores/config.test.ts index 553b467a..19fbebfd 100644 --- a/tests/stores/config.test.ts +++ b/tests/stores/config.test.ts @@ -534,6 +534,193 @@ describe("updateConfig — monitoredRepos pruning on selectedRepos change", () = }); }); +// ── customTabs orgScope/repoScope pruning on selectedRepos change ─────────── +// Same reconciliation bug class as monitoredRepos above: CustomTabModal's +// availableOrgs/availableRepos derive from selectedRepos (not selectedOrgs), +// so a custom tab's scope must be reconciled against selectedRepos, not +// against config.selectedOrgs. + +describe("updateConfig — customTabs scope pruning on selectedRepos change", () => { + beforeEach(() => { + resetConfig(); + }); + + function seedTab() { + updateConfig({ + selectedRepos: [ + { owner: "org", name: "a", fullName: "org/a" }, + { owner: "org", name: "b", fullName: "org/b" }, + { owner: "other", name: "c", fullName: "other/c" }, + ], + customTabs: [ + { + id: "t1", name: "Mixed Scope", baseType: "issues", + orgScope: ["org"], + repoScope: [{ owner: "other", name: "c", fullName: "other/c" }], + filterPreset: {}, exclusive: false, + }, + ], + }); + } + + it("prunes repoScope entries whose repo is no longer selected", () => { + createRoot((dispose) => { + seedTab(); + updateConfig({ + selectedRepos: [ + { owner: "org", name: "a", fullName: "org/a" }, + { owner: "org", name: "b", fullName: "org/b" }, + ], + }); + expect(config.customTabs[0].repoScope).toEqual([]); + // orgScope("org") is untouched — org/a and org/b are both still selected + expect(config.customTabs[0].orgScope).toEqual(["org"]); + dispose(); + }); + }); + + it("prunes orgScope entries once no selected repo has that owner", () => { + createRoot((dispose) => { + seedTab(); + updateConfig({ + selectedRepos: [{ owner: "other", name: "c", fullName: "other/c" }], + }); + expect(config.customTabs[0].orgScope).toEqual([]); + // repoScope(other/c) is untouched — still selected + expect(config.customTabs[0].repoScope).toEqual([ + { owner: "other", name: "c", fullName: "other/c" }, + ]); + dispose(); + }); + }); + + it("matches orgScope against selectedRepos owners case-insensitively", () => { + createRoot((dispose) => { + updateConfig({ + selectedRepos: [{ owner: "Org", name: "a", fullName: "Org/a" }], + customTabs: [{ + id: "t2", name: "Case Test", baseType: "issues", + orgScope: ["org"], repoScope: [], filterPreset: {}, exclusive: false, + }], + }); + // Re-apply the same selectedRepos (still "Org") — orgScope("org") must survive + updateConfig({ selectedRepos: [{ owner: "Org", name: "a", fullName: "Org/a" }] }); + expect(config.customTabs[0].orgScope).toEqual(["org"]); + dispose(); + }); + }); + + it("preserves object identity when pruning removes nothing", () => { + createRoot((dispose) => { + seedTab(); + const before = config.customTabs[0]; + // selectedRepos changes but every scoped repo/owner survives + updateConfig({ + selectedRepos: [ + { owner: "org", name: "a", fullName: "org/a" }, + { owner: "org", name: "b", fullName: "org/b" }, + { owner: "other", name: "c", fullName: "other/c" }, + { owner: "extra", name: "d", fullName: "extra/d" }, + ], + }); + expect(config.customTabs[0]).toBe(before); + dispose(); + }); + }); + + it("does not prune customTabs scope when selectedRepos is not in the update", () => { + createRoot((dispose) => { + seedTab(); + updateConfig({ theme: "dark" }); + expect(config.customTabs[0].orgScope).toEqual(["org"]); + expect(config.customTabs[0].repoScope).toEqual([ + { owner: "other", name: "c", fullName: "other/c" }, + ]); + dispose(); + }); + }); +}); + +// ── Structural invariant: repo-referencing config fields must reconcile +// against selectedRepos when it shrinks ────────────────────────────────────── +// +// Regression guard for a bug class where a persisted list references repos +// (or orgs derived from repos) but nothing prunes it when selectedRepos +// shrinks — the stale entry then survives forever (see the customTabs scope +// and monitoredRepos fixes above). Add a row to `dependentFields` for every +// NEW config field that stores a repo/org reference derived from +// selectedRepos — a missing row means a future field could reintroduce this +// bug class without any test catching it. +// +// Fields reviewed and confirmed INTENTIONALLY independent (do NOT need a row +// here — they are not derived from selectedRepos): +// - upstreamRepos: a disjoint category from selectedRepos (repos the user +// doesn't own), not a value derived from it — see discoverUpstreamRepos. +// - trackedUsers: references GitHub user logins, not repos/orgs; manually +// curated via explicit add/remove UI, never auto-merged from a live fetch. +// - selectedOrgs: reconciled against a live fetchOrgs() result rather than +// a sibling config field — covered separately in OrgSelector.test.tsx. +describe("updateConfig — structural invariant: repo-referencing fields reconcile with selectedRepos", () => { + beforeEach(() => { + resetConfig(); + }); + + const STALE_REPO = { owner: "stale", name: "repo", fullName: "stale/repo" }; + const SURVIVING_REPO = { owner: "keep", name: "repo", fullName: "keep/repo" }; + + const dependentFields: Array<{ name: string; seed: () => void; getValue: () => unknown }> = [ + { + name: "monitoredRepos", + seed: () => updateConfig({ + selectedRepos: [STALE_REPO, SURVIVING_REPO], + monitoredRepos: [STALE_REPO, SURVIVING_REPO], + }), + getValue: () => config.monitoredRepos, + }, + { + name: "customTabs[].repoScope", + seed: () => updateConfig({ + selectedRepos: [STALE_REPO, SURVIVING_REPO], + customTabs: [{ + id: "dep-repo-scope", name: "T", baseType: "issues", + orgScope: [], repoScope: [STALE_REPO, SURVIVING_REPO], + filterPreset: {}, exclusive: false, + }], + }), + getValue: () => config.customTabs[0].repoScope, + }, + { + name: "customTabs[].orgScope", + seed: () => updateConfig({ + selectedRepos: [STALE_REPO, SURVIVING_REPO], + customTabs: [{ + id: "dep-org-scope", name: "T", baseType: "issues", + orgScope: [STALE_REPO.owner, SURVIVING_REPO.owner], repoScope: [], + filterPreset: {}, exclusive: false, + }], + }), + getValue: () => config.customTabs[0].orgScope, + }, + ]; + + it.each(dependentFields.map((f) => [f.name, f] as const))( + "%s drops references to a repo removed from selectedRepos", + (_name, field) => { + createRoot((dispose) => { + field.seed(); + // Remove STALE_REPO from selectedRepos — every dependent field above + // referenced it and must no longer contain any trace of it. + updateConfig({ selectedRepos: [SURVIVING_REPO] }); + + const serialized = JSON.stringify(field.getValue()); + expect(serialized).not.toContain(STALE_REPO.owner); + expect(serialized).toContain(SURVIVING_REPO.owner); + dispose(); + }); + } + ); +}); + describe("setMonitoredRepo (C3)", () => { beforeEach(() => { resetConfig();