diff --git a/apps/web/app/settings/integrations/page.test.tsx b/apps/web/app/settings/integrations/page.test.tsx index 481545e899..fb42a81363 100644 --- a/apps/web/app/settings/integrations/page.test.tsx +++ b/apps/web/app/settings/integrations/page.test.tsx @@ -13,9 +13,18 @@ vi.mock("@/lib/routing/navigation-guard", () => ({ setNavigationBlocker: () => () => {}, })); -const { makeEnabledMock } = vi.hoisted(() => ({ - makeEnabledMock: (enabled: boolean) => () => ({ enabled, setEnabled: vi.fn(), loaded: true }), -})); +// The mocks record the workspace they were called with: the page's only job +// here is to hand each slider the workspace whose toggles it is editing. +const { makeEnabledMock, enabledCalls } = vi.hoisted(() => { + const enabledCalls: Array = []; + return { + enabledCalls, + makeEnabledMock: (enabled: boolean) => (workspaceId?: string | null) => { + enabledCalls.push(workspaceId); + return { enabled, setEnabled: vi.fn(), loaded: true }; + }, + }; +}); vi.mock("@/hooks/domains/azure-devops/use-azure-devops-enabled", () => ({ useAzureDevOpsEnabled: makeEnabledMock(true), @@ -50,6 +59,7 @@ function registerIntegration() { beforeEach(() => { pushNavigationStateSpy.mockClear(); + enabledCalls.length = 0; window.localStorage.removeItem("kandev:integrations:hideDisabledInNav:v1"); }); @@ -84,6 +94,15 @@ describe("IntegrationsIndexPage", () => { expect(ariaChecked(document.getElementById("github-enabled"))).toBe(ARIA_CHECKED_FALSE); }); + it("gives every slider the workspace whose toggles it edits", () => { + // The toggles are per workspace, so a slider that never learns its + // workspace would silently edit (and read) the wrong one. + renderPage("ws-42"); + + expect(enabledCalls).toHaveLength(6); + expect(new Set(enabledCalls)).toEqual(new Set(["ws-42"])); + }); + it("renders the hide-disabled-in-nav setting off by default and drafts a toggle without persisting until save", () => { renderPage(); diff --git a/apps/web/components/app-sidebar/sections/settings/settings-menu-branches.test.ts b/apps/web/components/app-sidebar/sections/settings/settings-menu-branches.test.ts index 62b4c360a1..e343e65f53 100644 --- a/apps/web/components/app-sidebar/sections/settings/settings-menu-branches.test.ts +++ b/apps/web/components/app-sidebar/sections/settings/settings-menu-branches.test.ts @@ -126,24 +126,41 @@ describe("buildWorkspacesBranch integration visibility", () => { const [workspace] = buildWorkspacesBranch( WORKSPACES, null, - new Set(["azure-devops", "linear"] as const), + () => new Set(["azure-devops", "linear"] as const), ); expect(integrationSlugsOf(workspace.children ?? [])).toEqual(["azure-devops", "linear"]); }); + it("resolves the visible set per workspace, since the toggles are per workspace", () => { + // Disabling GitHub in one workspace must not strip it from another's + // branch, which is exactly what a single shared set used to do. + const branch = buildWorkspacesBranch( + [ + { id: "ws-1", name: "First" }, + { id: "ws-2", name: "Second" }, + ], + null, + (workspaceId) => + workspaceId === "ws-1" ? new Set(["jira"] as const) : new Set(["github"] as const), + ); + + expect(integrationSlugsOf(branch[0].children ?? [])).toEqual(["jira"]); + expect(integrationSlugsOf(branch[1].children ?? [])).toEqual(["github"]); + }); + it("never consults whether an integration is configured — the badge owns that", () => { // Nothing about credentials reaches this builder: an integration the user // has never connected is listed exactly like a connected one, so the // visible set is the only thing that can remove a row. - const [workspace] = buildWorkspacesBranch(WORKSPACES, null, new Set(["github"] as const)); + const [workspace] = buildWorkspacesBranch(WORKSPACES, null, () => new Set(["github"] as const)); expect(integrationSlugsOf(workspace.children ?? [])).toEqual(["github"]); }); it("leaves the Integrations row navigable when the set hides all of them", () => { // An empty branch would otherwise render a chevron opening onto nothing. - const [workspace] = buildWorkspacesBranch(WORKSPACES, null, new Set()); + const [workspace] = buildWorkspacesBranch(WORKSPACES, null, () => new Set()); const integrations = integrationsTabOf(workspace.children ?? []); expect(integrations?.children).toEqual([]); diff --git a/apps/web/components/app-sidebar/sections/settings/settings-menu-branches.ts b/apps/web/components/app-sidebar/sections/settings/settings-menu-branches.ts index c80acc5bd9..b9132bf7d1 100644 --- a/apps/web/components/app-sidebar/sections/settings/settings-menu-branches.ts +++ b/apps/web/components/app-sidebar/sections/settings/settings-menu-branches.ts @@ -195,11 +195,13 @@ export function buildWorkspacesBranch( workspaces: ReadonlyArray, activeWorkspaceId?: string | null, /** - * Which integrations the Integrations branch may list. Omitted — the default - * — lists all of them; a set is passed only while "Hide disabled integrations - * from left panel navigation" is on. + * Which integrations a given workspace's Integrations branch may list. + * Returning `undefined` — the default — lists all of them; a set comes back + * only while "Hide disabled integrations from left panel navigation" is on. + * Resolved per workspace because the enable toggles are per workspace: one + * workspace hiding GitHub must not strip it from its siblings' branches. */ - visibleIntegrationSlugs?: ReadonlySet, + visibleIntegrationSlugsFor?: (workspaceId: string) => ReadonlySet | undefined, integrationContributions: ReadonlyArray = [], ): SettingsMenuNode[] { return workspaces.map((workspace) => { @@ -226,7 +228,7 @@ export function buildWorkspacesBranch( children: integrationNodes( workspace.id, integrationsHref, - visibleIntegrationSlugs, + visibleIntegrationSlugsFor?.(workspace.id), integrationContributions, ), integrationsWorkspaceId: workspace.id, diff --git a/apps/web/components/app-sidebar/sections/settings/use-settings-menu-branches.test.ts b/apps/web/components/app-sidebar/sections/settings/use-settings-menu-branches.test.ts index fa9666e96f..63fbe2c190 100644 --- a/apps/web/components/app-sidebar/sections/settings/use-settings-menu-branches.test.ts +++ b/apps/web/components/app-sidebar/sections/settings/use-settings-menu-branches.test.ts @@ -1,67 +1,67 @@ import { act, renderHook } from "@testing-library/react"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { pluginRegistry } from "@/lib/plugins/registry"; +import { + INTEGRATION_ENABLED_KEYS, + type IntegrationSlug, +} from "@/lib/integrations/integration-enabled-keys"; +import { makeLocalStorageMock } from "@/hooks/local-storage-mock.test-helpers"; import type { SettingsMenuNode } from "./settings-menu-branches"; const WORKSPACE_ID = "ws-1"; +const OTHER_WORKSPACE_ID = "ws-2"; const state = { workspaces: { activeId: WORKSPACE_ID, - items: [{ id: WORKSPACE_ID, name: "Main Workspace" }], + items: [ + { id: WORKSPACE_ID, name: "Main Workspace" }, + { id: OTHER_WORKSPACE_ID, name: "Second Workspace" }, + ], }, settingsAgents: { items: [] as unknown[] }, executors: { items: [] as unknown[] }, agentDiscovery: { items: [] as unknown[], loaded: false }, }; -// Per-integration enable/disable toggles plus the install-wide "hide disabled -// integrations from left panel navigation" setting, held as hoisted state so a -// test can flip either without re-mocking. -const integrationEnabled = vi.hoisted(() => ({ - "azure-devops": true, - github: true, - gitlab: true, - jira: true, - linear: true, - sentry: true, -})); +// The install-wide "hide disabled integrations from left panel navigation" +// setting, held as hoisted state so a test can flip it without re-mocking. The +// per-integration toggles are NOT mocked: they are per-workspace localStorage +// values, and reading them for real is the only way to prove the branch +// resolves each workspace's own. const hideDisabled = vi.hoisted(() => ({ value: false })); vi.mock("@/components/state-provider", () => ({ useAppStore: (selector: (s: typeof state) => unknown) => selector(state), })); -vi.mock("@/hooks/domains/azure-devops/use-azure-devops-enabled", () => ({ - useAzureDevOpsEnabled: () => ({ enabled: integrationEnabled["azure-devops"] }), -})); -vi.mock("@/hooks/domains/github/use-github-enabled", () => ({ - useGitHubEnabled: () => ({ enabled: integrationEnabled.github }), -})); -vi.mock("@/hooks/domains/gitlab/use-gitlab-enabled", () => ({ - useGitLabEnabled: () => ({ enabled: integrationEnabled.gitlab }), -})); -vi.mock("@/hooks/domains/jira/use-jira-enabled", () => ({ - useJiraEnabled: () => ({ enabled: integrationEnabled.jira }), -})); -vi.mock("@/hooks/domains/linear/use-linear-enabled", () => ({ - useLinearEnabled: () => ({ enabled: integrationEnabled.linear }), -})); -vi.mock("@/hooks/domains/sentry/use-sentry-enabled", () => ({ - useSentryEnabled: () => ({ enabled: integrationEnabled.sentry }), -})); vi.mock("@/hooks/domains/integrations/use-hide-disabled-integrations-in-nav", () => ({ useHideDisabledIntegrationsInNav: () => ({ hideDisabled: hideDisabled.value }), })); +const localStorageMock = makeLocalStorageMock(); +vi.stubGlobal("localStorage", localStorageMock); +Object.defineProperty(window, "localStorage", { + value: localStorageMock, + configurable: true, +}); + +/** Turn an integration off for one workspace, the way its slider does. */ +function disableIn(workspaceId: string, slug: IntegrationSlug): void { + window.localStorage.setItem( + `${INTEGRATION_ENABLED_KEYS[slug].storageKey}:${workspaceId}`, + "false", + ); +} + import { useSettingsMenuBranches } from "./use-settings-menu-branches"; const WORKSPACES_HREF = "/settings/workspaces"; -/** The integration slugs the Workspaces branch currently lists. */ -function listedIntegrations(): Array { +/** The integration slugs the Workspaces branch lists for one workspace. */ +function listedIntegrations(workspaceIndex = 0): Array { const { result } = renderHook(() => useSettingsMenuBranches("accordion")); - const workspace = result.current[WORKSPACES_HREF]?.children?.[0]; + const workspace = result.current[WORKSPACES_HREF]?.children?.[workspaceIndex]; const integrations = (workspace?.children ?? ([] as SettingsMenuNode[])).find((node) => node.href?.endsWith("/integrations"), ); @@ -71,24 +71,20 @@ function listedIntegrations(): Array { describe("useSettingsMenuBranches integration visibility", () => { beforeEach(() => { hideDisabled.value = false; - for (const slug of Object.keys(integrationEnabled) as Array) { - integrationEnabled[slug] = true; - } + localStorageMock.clear(); }); afterEach(() => { pluginRegistry.unregisterPlugin("plugin-bitbucket"); - // Restore the file-scoped mock state so no test inherits the previous one's + // Restore the file-scoped state so no test inherits the previous one's // toggles. hideDisabled.value = false; - for (const slug of Object.keys(integrationEnabled) as Array) { - integrationEnabled[slug] = true; - } + localStorageMock.clear(); }); it("lists every integration while the hide-disabled setting is off, disabled ones included", () => { - integrationEnabled.github = false; - integrationEnabled.sentry = false; + disableIn(WORKSPACE_ID, "github"); + disableIn(WORKSPACE_ID, "sentry"); expect(listedIntegrations()).toEqual([ "azure-devops", @@ -101,13 +97,23 @@ describe("useSettingsMenuBranches integration visibility", () => { }); it("hides the disabled integrations once the setting is on", () => { - integrationEnabled.github = false; - integrationEnabled.sentry = false; + disableIn(WORKSPACE_ID, "github"); + disableIn(WORKSPACE_ID, "sentry"); hideDisabled.value = true; expect(listedIntegrations()).toEqual(["azure-devops", "gitlab", "jira", "linear"]); }); + it("hides an integration only in the workspace that disabled it", () => { + // The toggle is per workspace: turning GitHub off in ws-1 must leave ws-2's + // branch untouched. + disableIn(WORKSPACE_ID, "github"); + hideDisabled.value = true; + + expect(listedIntegrations(0)).not.toContain("github"); + expect(listedIntegrations(1)).toContain("github"); + }); + it("keeps every integration when the setting is on but none are disabled", () => { hideDisabled.value = true; @@ -126,7 +132,6 @@ describe("useSettingsMenuBranches integration visibility", () => { // enabled-but-unconfigured integration stays reachable and only its badge // (resolved at render) reflects the connection. hideDisabled.value = true; - integrationEnabled.github = true; expect(listedIntegrations()).toContain("github"); }); @@ -156,7 +161,7 @@ describe("useSettingsMenuBranches integration visibility", () => { it("leaves the flat menu without branches at all, whatever the toggles say", () => { hideDisabled.value = true; - integrationEnabled.github = false; + disableIn(WORKSPACE_ID, "github"); const { result } = renderHook(() => useSettingsMenuBranches("flat")); diff --git a/apps/web/components/app-sidebar/sections/settings/use-settings-menu-branches.ts b/apps/web/components/app-sidebar/sections/settings/use-settings-menu-branches.ts index 49a574b866..354f076ba6 100644 --- a/apps/web/components/app-sidebar/sections/settings/use-settings-menu-branches.ts +++ b/apps/web/components/app-sidebar/sections/settings/use-settings-menu-branches.ts @@ -3,14 +3,13 @@ import { useMemo } from "react"; import { useAppStore } from "@/components/state-provider"; -import { useAzureDevOpsEnabled } from "@/hooks/domains/azure-devops/use-azure-devops-enabled"; -import { useGitHubEnabled } from "@/hooks/domains/github/use-github-enabled"; -import { useGitLabEnabled } from "@/hooks/domains/gitlab/use-gitlab-enabled"; import { useHideDisabledIntegrationsInNav } from "@/hooks/domains/integrations/use-hide-disabled-integrations-in-nav"; +import { useIntegrationEnabledReader } from "@/hooks/domains/integrations/use-integration-enabled"; import { useHideDisabledAgentProfilesInNav } from "@/hooks/domains/settings/use-hide-disabled-agent-profiles-in-nav"; -import { useJiraEnabled } from "@/hooks/domains/jira/use-jira-enabled"; -import { useLinearEnabled } from "@/hooks/domains/linear/use-linear-enabled"; -import { useSentryEnabled } from "@/hooks/domains/sentry/use-sentry-enabled"; +import { + INTEGRATION_ENABLED_KEYS, + INTEGRATION_ENABLED_SYNC_EVENTS, +} from "@/lib/integrations/integration-enabled-keys"; import { AGENTS_SETTINGS_HREF } from "@/lib/settings-discovery/catalog/agents"; import { EXECUTORS_SETTINGS_HREF } from "@/lib/settings-discovery/catalog/executors"; import { WORKSPACE_INTEGRATIONS } from "@/lib/settings-discovery/catalog/integrations"; @@ -63,7 +62,7 @@ export function useSettingsMenuBranches(mode: SettingsMenuMode): SettingsMenuBra // order. Before the scan hydrates this is empty and the saved order stands. const agentDiscovery = useAppStore((s) => s.agentDiscovery.items); const discoveryLoaded = useAppStore((s) => s.agentDiscovery.loaded); - const visibleIntegrations = useVisibleIntegrationSlugs(); + const visibleIntegrationsFor = useVisibleIntegrationSlugsFor(); const integrationContributions = pluginRegistry.getIntegrationSettings().map((integration) => ({ id: integration.id, label: integration.label, @@ -85,7 +84,7 @@ export function useSettingsMenuBranches(mode: SettingsMenuMode): SettingsMenuBra buildWorkspacesBranch( workspaces, activeWorkspaceId, - visibleIntegrations, + visibleIntegrationsFor, integrationContributions, ), ), @@ -103,15 +102,15 @@ export function useSettingsMenuBranches(mode: SettingsMenuMode): SettingsMenuBra executors, agentDiscovery, discoveryLoaded, - visibleIntegrations, + visibleIntegrationsFor, integrationContributions, hideDisabledAgentProfiles, ]); } /** - * Which integrations the Integrations branches may list, or `undefined` for all - * of them. + * Which integrations a workspace's Integrations branch may list, or `undefined` + * for all of them. * * The per-integration enable toggles gate **row visibility**, and only while * "Hide disabled integrations from left panel navigation" is on (off by @@ -121,42 +120,28 @@ export function useSettingsMenuBranches(mode: SettingsMenuMode): SettingsMenuBra * `configured && (!hideDisabled || enabled)`: the tree lists an integration you * have never connected, the sidebar nav does not. * - * Every toggle is a `localStorage`-backed `useSyncExternalStore` read, so this - * costs no requests; returning `undefined` on the default path keeps the - * builder from filtering at all. + * The branch lists every workspace, and the toggles are per workspace, so this + * resolves them per workspace rather than once — rules of hooks forbid calling + * `useXEnabled` in a loop, hence the subscribed reader over the key catalog. + * Every toggle is still a `localStorage` read, so this costs no requests; + * returning `undefined` on the default path keeps the builder from filtering at + * all. */ -function useVisibleIntegrationSlugs(): ReadonlySet | undefined { - const { enabled: azureDevOpsEnabled } = useAzureDevOpsEnabled(); - const { enabled: githubEnabled } = useGitHubEnabled(); - const { enabled: gitlabEnabled } = useGitLabEnabled(); - const { enabled: jiraEnabled } = useJiraEnabled(); - const { enabled: linearEnabled } = useLinearEnabled(); - const { enabled: sentryEnabled } = useSentryEnabled(); +function useVisibleIntegrationSlugsFor(): ( + workspaceId: string, +) => ReadonlySet | undefined { const { hideDisabled } = useHideDisabledIntegrationsInNav(); + const readEnabled = useIntegrationEnabledReader(INTEGRATION_ENABLED_SYNC_EVENTS); return useMemo(() => { - if (!hideDisabled) return undefined; - // A `Record` over the slug union rather than a list of spreads: a seventh - // integration then fails to compile here until it declares its toggle, - // instead of silently vanishing from the tree. - const enabled: Record = { - "azure-devops": azureDevOpsEnabled, - github: githubEnabled, - gitlab: gitlabEnabled, - jira: jiraEnabled, - linear: linearEnabled, - sentry: sentryEnabled, - }; - return new Set(WORKSPACE_INTEGRATIONS.map(([slug]) => slug).filter((slug) => enabled[slug])); - }, [ - hideDisabled, - azureDevOpsEnabled, - githubEnabled, - gitlabEnabled, - jiraEnabled, - linearEnabled, - sentryEnabled, - ]); + if (!hideDisabled) return () => undefined; + return (workspaceId: string) => + new Set( + WORKSPACE_INTEGRATIONS.map(([slug]) => slug).filter((slug) => + readEnabled(INTEGRATION_ENABLED_KEYS[slug], workspaceId), + ), + ); + }, [hideDisabled, readEnabled]); } /** diff --git a/apps/web/components/azure-devops/azure-devops-enabled-control.tsx b/apps/web/components/azure-devops/azure-devops-enabled-control.tsx index accb166e8a..67b5c1210a 100644 --- a/apps/web/components/azure-devops/azure-devops-enabled-control.tsx +++ b/apps/web/components/azure-devops/azure-devops-enabled-control.tsx @@ -2,10 +2,14 @@ import { DraftedIntegrationEnabledControl } from "@/components/integrations/drafted-integration-enabled-control"; import { useAzureDevOpsEnabled } from "@/hooks/domains/azure-devops/use-azure-devops-enabled"; +import type { IntegrationEnabledControlProps } from "@/components/integrations/integration-enabled-control-props"; -/** Enable/disable slider for the Azure DevOps integration, wired to `useAzureDevOpsEnabled`. */ -export function AzureDevOpsEnabledControl() { - const { enabled, setEnabled } = useAzureDevOpsEnabled(); +/** + * Enable/disable slider for the Azure DevOps integration in `workspaceId`, wired to + * `useAzureDevOpsEnabled`. + */ +export function AzureDevOpsEnabledControl({ workspaceId }: IntegrationEnabledControlProps) { + const { enabled, setEnabled } = useAzureDevOpsEnabled(workspaceId); return ( } title={t("azuredevops:integrationTitle")} description={t("azuredevops:integrationDescription")} - action={} + action={} > diff --git a/apps/web/components/github/github-enabled-control.tsx b/apps/web/components/github/github-enabled-control.tsx index 8fa063284f..0cb88d9c4b 100644 --- a/apps/web/components/github/github-enabled-control.tsx +++ b/apps/web/components/github/github-enabled-control.tsx @@ -2,10 +2,14 @@ import { DraftedIntegrationEnabledControl } from "@/components/integrations/drafted-integration-enabled-control"; import { useGitHubEnabled } from "@/hooks/domains/github/use-github-enabled"; +import type { IntegrationEnabledControlProps } from "@/components/integrations/integration-enabled-control-props"; -/** Enable/disable slider for the GitHub integration, wired to `useGitHubEnabled`. */ -export function GitHubEnabledControl() { - const { enabled, setEnabled } = useGitHubEnabled(); +/** + * Enable/disable slider for the GitHub integration in `workspaceId`, wired to + * `useGitHubEnabled`. + */ +export function GitHubEnabledControl({ workspaceId }: IntegrationEnabledControlProps) { + const { enabled, setEnabled } = useGitHubEnabled(workspaceId); return ( } + action={} > } action={
- +
diff --git a/apps/web/components/integrations/integrations-menu.test.ts b/apps/web/components/integrations/integrations-menu.test.ts index f5bd385ef7..1837224458 100644 --- a/apps/web/components/integrations/integrations-menu.test.ts +++ b/apps/web/components/integrations/integrations-menu.test.ts @@ -36,7 +36,7 @@ vi.mock("@/hooks/domains/linear/use-linear-availability", () => ({ useLinearAuthed: useLinearAuthedMock, })); -// useNavAvailability now also folds in each integration's install-wide +// useNavAvailability now also folds in each integration's per-workspace // enabled toggle plus the "hide disabled" nav setting. This suite never // exercises that decoupling (see hooks/use-nav-availability.test.ts for // that) — every enabled hook here defaults to `true` and the hide-disabled diff --git a/apps/web/components/jira/jira-enabled-control.tsx b/apps/web/components/jira/jira-enabled-control.tsx index 5d90e09268..7f6648e014 100644 --- a/apps/web/components/jira/jira-enabled-control.tsx +++ b/apps/web/components/jira/jira-enabled-control.tsx @@ -2,10 +2,14 @@ import { DraftedIntegrationEnabledControl } from "@/components/integrations/drafted-integration-enabled-control"; import { useJiraEnabled } from "@/hooks/domains/jira/use-jira-enabled"; +import type { IntegrationEnabledControlProps } from "@/components/integrations/integration-enabled-control-props"; -/** Enable/disable slider for the Jira integration, wired to `useJiraEnabled`. */ -export function JiraEnabledControl() { - const { enabled, setEnabled } = useJiraEnabled(); +/** + * Enable/disable slider for the Jira integration in `workspaceId`, wired to + * `useJiraEnabled`. + */ +export function JiraEnabledControl({ workspaceId }: IntegrationEnabledControlProps) { + const { enabled, setEnabled } = useJiraEnabled(workspaceId); return ( } title={t("jira:jiraIntegration")} description={t("jira:connectThisWorkspaceToAtlassianCloud")} - action={} + action={} > diff --git a/apps/web/components/linear/linear-enabled-control.tsx b/apps/web/components/linear/linear-enabled-control.tsx index 6cc0321ef7..b861960807 100644 --- a/apps/web/components/linear/linear-enabled-control.tsx +++ b/apps/web/components/linear/linear-enabled-control.tsx @@ -2,10 +2,14 @@ import { DraftedIntegrationEnabledControl } from "@/components/integrations/drafted-integration-enabled-control"; import { useLinearEnabled } from "@/hooks/domains/linear/use-linear-enabled"; +import type { IntegrationEnabledControlProps } from "@/components/integrations/integration-enabled-control-props"; -/** Enable/disable slider for the Linear integration, wired to `useLinearEnabled`. */ -export function LinearEnabledControl() { - const { enabled, setEnabled } = useLinearEnabled(); +/** + * Enable/disable slider for the Linear integration in `workspaceId`, wired to + * `useLinearEnabled`. + */ +export function LinearEnabledControl({ workspaceId }: IntegrationEnabledControlProps) { + const { enabled, setEnabled } = useLinearEnabled(workspaceId); return ( } title={t("linear:linearIntegration")} description={t("linear:linearIntegrationDescription")} - action={} + action={} > diff --git a/apps/web/components/sentry/sentry-enabled-control.tsx b/apps/web/components/sentry/sentry-enabled-control.tsx index 6f01879448..ab90468f41 100644 --- a/apps/web/components/sentry/sentry-enabled-control.tsx +++ b/apps/web/components/sentry/sentry-enabled-control.tsx @@ -2,10 +2,14 @@ import { DraftedIntegrationEnabledControl } from "@/components/integrations/drafted-integration-enabled-control"; import { useSentryEnabled } from "@/hooks/domains/sentry/use-sentry-enabled"; +import type { IntegrationEnabledControlProps } from "@/components/integrations/integration-enabled-control-props"; -/** Enable/disable slider for the Sentry integration, wired to `useSentryEnabled`. */ -export function SentryEnabledControl() { - const { enabled, setEnabled } = useSentryEnabled(); +/** + * Enable/disable slider for the Sentry integration in `workspaceId`, wired to + * `useSentryEnabled`. + */ +export function SentryEnabledControl({ workspaceId }: IntegrationEnabledControlProps) { + const { enabled, setEnabled } = useSentryEnabled(workspaceId); return ( } title={t("sentry:sentryIntegration")} description={t("sentry:sentryIntegrationDescription")} - action={} + action={} > diff --git a/apps/web/e2e/tests/integrations/integrations-enabled-per-workspace.spec.ts b/apps/web/e2e/tests/integrations/integrations-enabled-per-workspace.spec.ts new file mode 100644 index 0000000000..af03cf3793 --- /dev/null +++ b/apps/web/e2e/tests/integrations/integrations-enabled-per-workspace.spec.ts @@ -0,0 +1,54 @@ +import { test, expect } from "../../fixtures/test-base"; + +// Covers docs/specs/integrations/enable-disable-toggle.md's per-workspace +// scoping: the enable toggle belongs to the workspace whose settings page it +// sits on, so disabling an integration in one workspace leaves every other +// workspace's toggle alone. It shipped install-wide first, which silenced an +// integration everywhere from a single workspace's slider. +test.describe("integration enable toggles are scoped to their workspace", () => { + test("disabling GitHub in one workspace leaves the other workspace enabled", async ({ + testPage, + apiClient, + prCapture, + }) => { + const first = await apiClient.createWorkspace("Toggle Scope A"); + const second = await apiClient.createWorkspace("Toggle Scope B"); + + const firstIntegrations = `/settings/workspaces/${first.id}/integrations`; + const secondIntegrations = `/settings/workspaces/${second.id}/integrations`; + + await testPage.goto(firstIntegrations); + const githubSwitch = testPage.locator("#github-enabled"); + await expect(githubSwitch).toHaveAttribute("aria-checked", "true"); + await githubSwitch.click(); + await testPage + .getByTestId("settings-floating-save") + .getByRole("button", { name: "Save changes" }) + .click(); + await expect(testPage.getByTestId("settings-floating-save")).not.toBeVisible(); + + await prCapture.screenshot("workspace-a-github-disabled", { + caption: "Workspace A: GitHub disabled", + }); + + // The other workspace never asked for GitHub to be off. + await testPage.goto(secondIntegrations); + await expect(testPage.locator("#github-enabled")).toHaveAttribute("aria-checked", "true"); + await prCapture.screenshot("workspace-b-github-still-enabled", { + caption: "Workspace B: GitHub still enabled", + }); + + // ...and GitHub's own settings page under that workspace agrees. + await testPage.goto(`${secondIntegrations}/github`); + await expect(testPage.locator("#github-enabled")).toHaveAttribute("aria-checked", "true"); + await prCapture.screenshot("workspace-b-github-page-enabled", { + caption: "Workspace B, GitHub settings page: still enabled", + }); + + // The workspace that disabled it still reads disabled on both surfaces. + await testPage.goto(firstIntegrations); + await expect(testPage.locator("#github-enabled")).toHaveAttribute("aria-checked", "false"); + await testPage.goto(`${firstIntegrations}/github`); + await expect(testPage.locator("#github-enabled")).toHaveAttribute("aria-checked", "false"); + }); +}); diff --git a/apps/web/e2e/tests/integrations/integrations-index-enabled-toggle.spec.ts b/apps/web/e2e/tests/integrations/integrations-index-enabled-toggle.spec.ts index 534f7aefde..9f03159297 100644 --- a/apps/web/e2e/tests/integrations/integrations-index-enabled-toggle.spec.ts +++ b/apps/web/e2e/tests/integrations/integrations-index-enabled-toggle.spec.ts @@ -3,7 +3,7 @@ import { test, expect } from "../../fixtures/test-base"; // Covers docs/specs/integrations/enable-disable-toggle.md's index-page slider // scenarios: every integration row has a working slider, toggling it never // navigates, and the toggle stays in sync with that integration's own -// settings page (the shared install-wide `useXEnabled` state). +// settings page (the shared per-workspace `useXEnabled` state). test.describe("integrations index page enable/disable sliders", () => { test("every integration row has a slider, toggling it does not navigate, and it stays in sync with the own-page slider", async ({ testPage, diff --git a/apps/web/hooks/domains/azure-devops/use-azure-devops-enabled.test.ts b/apps/web/hooks/domains/azure-devops/use-azure-devops-enabled.test.ts index 5ff620b755..5026057bd4 100644 --- a/apps/web/hooks/domains/azure-devops/use-azure-devops-enabled.test.ts +++ b/apps/web/hooks/domains/azure-devops/use-azure-devops-enabled.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import { act, renderHook, waitFor } from "@testing-library/react"; import { useAzureDevOpsEnabled } from "./use-azure-devops-enabled"; import { makeLocalStorageMock } from "../../local-storage-mock.test-helpers"; +import { resetIntegrationEnabledMigrations } from "../integrations/use-integration-enabled"; const STORAGE_KEY = "kandev:azure-devops:enabled:v1"; @@ -15,6 +16,7 @@ Object.defineProperty(window, "localStorage", { describe("useAzureDevOpsEnabled", () => { beforeEach(() => { localStorageMock.clear(); + resetIntegrationEnabledMigrations(); }); afterEach(() => { localStorageMock.clear(); @@ -68,14 +70,19 @@ describe("useAzureDevOpsEnabled", () => { await waitFor(() => expect(result.current.enabled).toBe(false)); }); - it("migrates a legacy per-workspace key onto the canonical storage key", async () => { - window.localStorage.setItem("kandev:azure-devops:enabled:ws-123", "false"); + it("restores a legacy per-workspace key to that workspace's own key", async () => { + // The pre-scoping keys were `kandev:azure-devops:enabled::v1`, one per + // workspace, so each is restored to the workspace it came from rather than + // folded into a single install-wide value. + window.localStorage.setItem("kandev:azure-devops:enabled:ws-123:v1", "false"); - const { result } = renderHook(() => useAzureDevOpsEnabled()); + const { result } = renderHook(() => useAzureDevOpsEnabled("ws-123")); await waitFor(() => expect(result.current.loaded).toBe(true)); expect(result.current.enabled).toBe(false); - expect(window.localStorage.getItem(STORAGE_KEY)).toBe("false"); - expect(window.localStorage.getItem("kandev:azure-devops:enabled:ws-123")).toBeNull(); + expect(window.localStorage.getItem(`${STORAGE_KEY}:ws-123`)).toBe("false"); + expect(window.localStorage.getItem("kandev:azure-devops:enabled:ws-123:v1")).toBeNull(); + // Another workspace is untouched by that migration. + expect(renderHook(() => useAzureDevOpsEnabled("ws-other")).result.current.enabled).toBe(true); }); }); diff --git a/apps/web/hooks/domains/azure-devops/use-azure-devops-enabled.ts b/apps/web/hooks/domains/azure-devops/use-azure-devops-enabled.ts index 92e36207aa..276d481640 100644 --- a/apps/web/hooks/domains/azure-devops/use-azure-devops-enabled.ts +++ b/apps/web/hooks/domains/azure-devops/use-azure-devops-enabled.ts @@ -1,18 +1,18 @@ "use client"; +import { INTEGRATION_ENABLED_KEYS } from "@/lib/integrations/integration-enabled-keys"; import { useIntegrationEnabled } from "../integrations/use-integration-enabled"; -const STORAGE_KEY = "kandev:azure-devops:enabled:v1"; -const LEGACY_KEY_PREFIX = "kandev:azure-devops:enabled:"; -const SYNC_EVENT = "kandev:azure-devops:enabled-changed"; +const { storageKey, legacyKeyPrefix, syncEvent } = INTEGRATION_ENABLED_KEYS["azure-devops"]; /** - * Install-wide enable/disable state for the Azure DevOps integration. + * Per-workspace enable/disable state for the Azure DevOps integration. * - * Backed by `localStorage` (key `kandev:azure-devops:enabled:v1`), synced - * across browser tabs and across the own-settings-page and index-page - * sliders. Defaults to `true` when no value has ever been persisted. + * Backed by `localStorage` (key `kandev:azure-devops:enabled:v1:`), synced across + * browser tabs and across the own-settings-page and index-page sliders. + * Defaults to `true` when no value has ever been persisted for the workspace. + * Omitting `workspaceId` reads the pre-scoping install-wide value. */ -export function useAzureDevOpsEnabled() { - return useIntegrationEnabled(STORAGE_KEY, LEGACY_KEY_PREFIX, SYNC_EVENT); +export function useAzureDevOpsEnabled(workspaceId?: string | null) { + return useIntegrationEnabled(storageKey, legacyKeyPrefix, syncEvent, workspaceId); } diff --git a/apps/web/hooks/domains/github/use-github-enabled.test.ts b/apps/web/hooks/domains/github/use-github-enabled.test.ts index 0cd91486da..2af1afa0c7 100644 --- a/apps/web/hooks/domains/github/use-github-enabled.test.ts +++ b/apps/web/hooks/domains/github/use-github-enabled.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import { act, renderHook, waitFor } from "@testing-library/react"; import { useGitHubEnabled } from "./use-github-enabled"; import { makeLocalStorageMock } from "../../local-storage-mock.test-helpers"; +import { resetIntegrationEnabledMigrations } from "../integrations/use-integration-enabled"; const STORAGE_KEY = "kandev:github:enabled:v1"; @@ -15,6 +16,7 @@ Object.defineProperty(window, "localStorage", { describe("useGitHubEnabled", () => { beforeEach(() => { localStorageMock.clear(); + resetIntegrationEnabledMigrations(); }); afterEach(() => { localStorageMock.clear(); @@ -68,14 +70,19 @@ describe("useGitHubEnabled", () => { await waitFor(() => expect(result.current.enabled).toBe(false)); }); - it("migrates a legacy per-workspace key onto the canonical storage key", async () => { - window.localStorage.setItem("kandev:github:enabled:ws-123", "false"); + it("restores a legacy per-workspace key to that workspace's own key", async () => { + // The pre-scoping keys were `kandev:github:enabled::v1`, one per + // workspace, so each is restored to the workspace it came from rather than + // folded into a single install-wide value. + window.localStorage.setItem("kandev:github:enabled:ws-123:v1", "false"); - const { result } = renderHook(() => useGitHubEnabled()); + const { result } = renderHook(() => useGitHubEnabled("ws-123")); await waitFor(() => expect(result.current.loaded).toBe(true)); expect(result.current.enabled).toBe(false); - expect(window.localStorage.getItem(STORAGE_KEY)).toBe("false"); - expect(window.localStorage.getItem("kandev:github:enabled:ws-123")).toBeNull(); + expect(window.localStorage.getItem(`${STORAGE_KEY}:ws-123`)).toBe("false"); + expect(window.localStorage.getItem("kandev:github:enabled:ws-123:v1")).toBeNull(); + // Another workspace is untouched by that migration. + expect(renderHook(() => useGitHubEnabled("ws-other")).result.current.enabled).toBe(true); }); }); diff --git a/apps/web/hooks/domains/github/use-github-enabled.ts b/apps/web/hooks/domains/github/use-github-enabled.ts index 6a0754abe1..b3dc73b7df 100644 --- a/apps/web/hooks/domains/github/use-github-enabled.ts +++ b/apps/web/hooks/domains/github/use-github-enabled.ts @@ -1,18 +1,18 @@ "use client"; +import { INTEGRATION_ENABLED_KEYS } from "@/lib/integrations/integration-enabled-keys"; import { useIntegrationEnabled } from "../integrations/use-integration-enabled"; -const STORAGE_KEY = "kandev:github:enabled:v1"; -const LEGACY_KEY_PREFIX = "kandev:github:enabled:"; -const SYNC_EVENT = "kandev:github:enabled-changed"; +const { storageKey, legacyKeyPrefix, syncEvent } = INTEGRATION_ENABLED_KEYS["github"]; /** - * Install-wide enable/disable state for the GitHub integration. + * Per-workspace enable/disable state for the GitHub integration. * - * Backed by `localStorage` (key `kandev:github:enabled:v1`), synced across + * Backed by `localStorage` (key `kandev:github:enabled:v1:`), synced across * browser tabs and across the own-settings-page and index-page sliders. - * Defaults to `true` when no value has ever been persisted. + * Defaults to `true` when no value has ever been persisted for the workspace. + * Omitting `workspaceId` reads the pre-scoping install-wide value. */ -export function useGitHubEnabled() { - return useIntegrationEnabled(STORAGE_KEY, LEGACY_KEY_PREFIX, SYNC_EVENT); +export function useGitHubEnabled(workspaceId?: string | null) { + return useIntegrationEnabled(storageKey, legacyKeyPrefix, syncEvent, workspaceId); } diff --git a/apps/web/hooks/domains/gitlab/use-gitlab-enabled.test.ts b/apps/web/hooks/domains/gitlab/use-gitlab-enabled.test.ts index 1f9e56b7ba..a4666fd285 100644 --- a/apps/web/hooks/domains/gitlab/use-gitlab-enabled.test.ts +++ b/apps/web/hooks/domains/gitlab/use-gitlab-enabled.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import { act, renderHook, waitFor } from "@testing-library/react"; import { useGitLabEnabled } from "./use-gitlab-enabled"; import { makeLocalStorageMock } from "../../local-storage-mock.test-helpers"; +import { resetIntegrationEnabledMigrations } from "../integrations/use-integration-enabled"; const STORAGE_KEY = "kandev:gitlab:enabled:v1"; @@ -15,6 +16,7 @@ Object.defineProperty(window, "localStorage", { describe("useGitLabEnabled", () => { beforeEach(() => { localStorageMock.clear(); + resetIntegrationEnabledMigrations(); }); afterEach(() => { localStorageMock.clear(); @@ -68,14 +70,19 @@ describe("useGitLabEnabled", () => { await waitFor(() => expect(result.current.enabled).toBe(false)); }); - it("migrates a legacy per-workspace key onto the canonical storage key", async () => { - window.localStorage.setItem("kandev:gitlab:enabled:ws-123", "false"); + it("restores a legacy per-workspace key to that workspace's own key", async () => { + // The pre-scoping keys were `kandev:gitlab:enabled::v1`, one per + // workspace, so each is restored to the workspace it came from rather than + // folded into a single install-wide value. + window.localStorage.setItem("kandev:gitlab:enabled:ws-123:v1", "false"); - const { result } = renderHook(() => useGitLabEnabled()); + const { result } = renderHook(() => useGitLabEnabled("ws-123")); await waitFor(() => expect(result.current.loaded).toBe(true)); expect(result.current.enabled).toBe(false); - expect(window.localStorage.getItem(STORAGE_KEY)).toBe("false"); - expect(window.localStorage.getItem("kandev:gitlab:enabled:ws-123")).toBeNull(); + expect(window.localStorage.getItem(`${STORAGE_KEY}:ws-123`)).toBe("false"); + expect(window.localStorage.getItem("kandev:gitlab:enabled:ws-123:v1")).toBeNull(); + // Another workspace is untouched by that migration. + expect(renderHook(() => useGitLabEnabled("ws-other")).result.current.enabled).toBe(true); }); }); diff --git a/apps/web/hooks/domains/gitlab/use-gitlab-enabled.ts b/apps/web/hooks/domains/gitlab/use-gitlab-enabled.ts index 253a950db4..609d3a84ae 100644 --- a/apps/web/hooks/domains/gitlab/use-gitlab-enabled.ts +++ b/apps/web/hooks/domains/gitlab/use-gitlab-enabled.ts @@ -1,18 +1,18 @@ "use client"; +import { INTEGRATION_ENABLED_KEYS } from "@/lib/integrations/integration-enabled-keys"; import { useIntegrationEnabled } from "../integrations/use-integration-enabled"; -const STORAGE_KEY = "kandev:gitlab:enabled:v1"; -const LEGACY_KEY_PREFIX = "kandev:gitlab:enabled:"; -const SYNC_EVENT = "kandev:gitlab:enabled-changed"; +const { storageKey, legacyKeyPrefix, syncEvent } = INTEGRATION_ENABLED_KEYS["gitlab"]; /** - * Install-wide enable/disable state for the GitLab integration. + * Per-workspace enable/disable state for the GitLab integration. * - * Backed by `localStorage` (key `kandev:gitlab:enabled:v1`), synced across + * Backed by `localStorage` (key `kandev:gitlab:enabled:v1:`), synced across * browser tabs and across the own-settings-page and index-page sliders. - * Defaults to `true` when no value has ever been persisted. + * Defaults to `true` when no value has ever been persisted for the workspace. + * Omitting `workspaceId` reads the pre-scoping install-wide value. */ -export function useGitLabEnabled() { - return useIntegrationEnabled(STORAGE_KEY, LEGACY_KEY_PREFIX, SYNC_EVENT); +export function useGitLabEnabled(workspaceId?: string | null) { + return useIntegrationEnabled(storageKey, legacyKeyPrefix, syncEvent, workspaceId); } diff --git a/apps/web/hooks/domains/integrations/use-enabled-integrations.ts b/apps/web/hooks/domains/integrations/use-enabled-integrations.ts index 56fde9ee77..b5d9cb7337 100644 --- a/apps/web/hooks/domains/integrations/use-enabled-integrations.ts +++ b/apps/web/hooks/domains/integrations/use-enabled-integrations.ts @@ -24,8 +24,8 @@ export type IntegrationSlug = (typeof WORKSPACE_INTEGRATIONS)[number][0]; * The predicates are the ones the settings tree used before it flattened into * a menu, kept identical so the badge still means what it used to. Note the * asymmetry that carries over with them: most read the connection alone, while - * Sentry's `available` also requires its per-workspace toggle, so a connected - * Sentry whose toggle is off reports false. + * Sentry's `available` also requires that workspace's enable toggle, so a + * connected Sentry whose toggle is off reports false. */ export function useEnabledIntegrations(workspaceId: string): ReadonlySet { const azureDevOps = useAzureDevOpsAvailable(workspaceId); diff --git a/apps/web/hooks/domains/integrations/use-integration-availability.ts b/apps/web/hooks/domains/integrations/use-integration-availability.ts index c9cb5f6181..2f54c56494 100644 --- a/apps/web/hooks/domains/integrations/use-integration-availability.ts +++ b/apps/web/hooks/domains/integrations/use-integration-availability.ts @@ -19,7 +19,7 @@ export type IntegrationConfigStatus = { lastOk?: boolean; }; -// Reads the backend-recorded auth health for the install-wide integration. +// Reads the backend-recorded auth health for the integration. // Returns true only when a config exists, has a secret, and the most recent // probe succeeded. Pass `active=false` to skip fetching entirely (e.g. while // the user toggle is off) — this avoids the polling overhead on disabled @@ -70,24 +70,25 @@ export function useIntegrationAuthed( } export type IntegrationAvailabilityOptions = { - // Install-wide enabled toggle that has settled. `loaded` gates the - // probe so we don't waste a fetch on the first render when the toggle is - // off. - useEnabled: () => { enabled: boolean; loaded: boolean }; + // The workspace's enabled toggle, already read by the caller (it owns the + // workspace id its `fetchConfig` is keyed by, so it owns the toggle read + // too). `loaded` gates the probe so we don't waste a fetch on the first + // render when the toggle is off. + enabledState: { enabled: boolean; loaded: boolean }; fetchConfig: () => Promise; refreshMs?: number; }; -// Combined check for showing an integration's UI: the user toggle is on AND -// the backend reports a configured, healthy connection. When the toggle is +// Combined check for showing an integration's UI: the workspace's toggle is on +// AND the backend reports a configured, healthy connection. When the toggle is // off (or hasn't loaded yet) the auth probe is skipped — disabled // integrations don't poll the backend. export function useIntegrationAvailable({ - useEnabled, + enabledState, fetchConfig, refreshMs, }: IntegrationAvailabilityOptions): boolean { - const { enabled, loaded } = useEnabled(); + const { enabled, loaded } = enabledState; const active = loaded && enabled; const authed = useIntegrationAuthed(fetchConfig, refreshMs, active); return active && authed; diff --git a/apps/web/hooks/domains/integrations/use-integration-enabled.test.ts b/apps/web/hooks/domains/integrations/use-integration-enabled.test.ts new file mode 100644 index 0000000000..75d700870a --- /dev/null +++ b/apps/web/hooks/domains/integrations/use-integration-enabled.test.ts @@ -0,0 +1,230 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import { act, renderHook } from "@testing-library/react"; +import { makeLocalStorageMock } from "../../local-storage-mock.test-helpers"; +import { + integrationEnabledStorageKey, + readIntegrationEnabled, + resetIntegrationEnabledMigrations, + useIntegrationEnabled, + useIntegrationEnabledReader, +} from "./use-integration-enabled"; + +const STORAGE_KEY = "kandev:acme:enabled:v1"; +const LEGACY_PREFIX = "kandev:acme:enabled:"; +const SYNC_EVENT = "kandev:acme:enabled-changed"; +const KEYS = { storageKey: STORAGE_KEY, legacyKeyPrefix: LEGACY_PREFIX }; +// The pre-scoping key shape, workspace id in the middle: see the versions of +// `use-jira-enabled.ts` before the toggle was made install-wide. +const legacyKey = (workspaceId: string) => `${LEGACY_PREFIX}${workspaceId}:v1`; +const WORKSPACE_A = "ws-a"; +const WORKSPACE_B = "ws-b"; + +const localStorageMock = makeLocalStorageMock(); +vi.stubGlobal("localStorage", localStorageMock); +Object.defineProperty(window, "localStorage", { + value: localStorageMock, + configurable: true, +}); + +function renderEnabled(workspaceId?: string | null) { + return renderHook(() => + useIntegrationEnabled(STORAGE_KEY, LEGACY_PREFIX, SYNC_EVENT, workspaceId), + ); +} + +describe("useIntegrationEnabled workspace scoping", () => { + beforeEach(() => { + localStorageMock.clear(); + resetIntegrationEnabledMigrations(); + }); + afterEach(() => localStorageMock.clear()); + + it("writes each workspace's toggle to its own key", () => { + const { result } = renderEnabled(WORKSPACE_A); + + act(() => result.current.setEnabled(false)); + + expect(window.localStorage.getItem(`${STORAGE_KEY}:${WORKSPACE_A}`)).toBe("false"); + // The pre-scoping install-wide key is never written, so it keeps working as + // the seed for workspaces that have not been toggled. + expect(window.localStorage.getItem(STORAGE_KEY)).toBeNull(); + }); + + it("leaves the other workspaces enabled when one turns the integration off", () => { + const a = renderEnabled(WORKSPACE_A); + act(() => a.result.current.setEnabled(false)); + + const b = renderEnabled(WORKSPACE_B); + + expect(a.result.current.enabled).toBe(false); + expect(b.result.current.enabled).toBe(true); + }); + + it("re-reads for the workspace it is given when the active workspace changes", () => { + window.localStorage.setItem(`${STORAGE_KEY}:${WORKSPACE_B}`, "false"); + const { result, rerender } = renderHook( + ({ workspaceId }: { workspaceId: string }) => + useIntegrationEnabled(STORAGE_KEY, LEGACY_PREFIX, SYNC_EVENT, workspaceId), + { initialProps: { workspaceId: WORKSPACE_A } }, + ); + expect(result.current.enabled).toBe(true); + + rerender({ workspaceId: WORKSPACE_B }); + + expect(result.current.enabled).toBe(false); + }); + + it("seeds an untouched workspace from the pre-scoping install-wide value", () => { + // Upgrading must not silently re-enable an integration the user had turned + // off while the toggle was install-wide. + window.localStorage.setItem(STORAGE_KEY, "false"); + + const { result } = renderEnabled(WORKSPACE_A); + + expect(result.current.enabled).toBe(false); + }); + + it("lets a workspace override the install-wide value it was seeded from", () => { + window.localStorage.setItem(STORAGE_KEY, "false"); + const a = renderEnabled(WORKSPACE_A); + + act(() => a.result.current.setEnabled(true)); + + expect(a.result.current.enabled).toBe(true); + expect(renderEnabled(WORKSPACE_B).result.current.enabled).toBe(false); + }); + + it("defaults to enabled with no stored value at either scope", () => { + expect(renderEnabled(WORKSPACE_A).result.current.enabled).toBe(true); + }); + + it("reads and writes the bare key when no workspace is known", () => { + const { result } = renderEnabled(undefined); + + act(() => result.current.setEnabled(false)); + + expect(window.localStorage.getItem(STORAGE_KEY)).toBe("false"); + }); + + it("restores each pre-v1 legacy key to its own workspace", () => { + // The pre-v1 keys were already per workspace. Folding them into one value + // would recreate exactly the bug this scoping fixes, so each is restored to + // the workspace it came from. + window.localStorage.setItem(legacyKey(WORKSPACE_A), "false"); + window.localStorage.setItem(legacyKey(WORKSPACE_B), "true"); + + const a = renderEnabled(WORKSPACE_A); + + expect(a.result.current.enabled).toBe(false); + expect(renderEnabled(WORKSPACE_B).result.current.enabled).toBe(true); + expect(window.localStorage.getItem(`${STORAGE_KEY}:${WORKSPACE_A}`)).toBe("false"); + expect(window.localStorage.getItem(legacyKey(WORKSPACE_A))).toBeNull(); + expect(window.localStorage.getItem(legacyKey(WORKSPACE_B))).toBeNull(); + // The install-wide key is never invented from a workspace's value. + expect(window.localStorage.getItem(STORAGE_KEY)).toBeNull(); + }); + + it("never migrates over a value the workspace already has", () => { + window.localStorage.setItem(legacyKey(WORKSPACE_A), "false"); + window.localStorage.setItem(`${STORAGE_KEY}:${WORKSPACE_A}`, "true"); + + const { result } = renderEnabled(WORKSPACE_A); + + expect(result.current.enabled).toBe(true); + expect(window.localStorage.getItem(legacyKey(WORKSPACE_A))).toBeNull(); + }); + + it("leaves the current per-workspace keys alone, though they share the prefix", () => { + window.localStorage.setItem(`${STORAGE_KEY}:${WORKSPACE_B}`, "false"); + + renderEnabled(WORKSPACE_A); + + expect(window.localStorage.getItem(`${STORAGE_KEY}:${WORKSPACE_B}`)).toBe("false"); + }); + + it("propagates a same-tab toggle to the other slider on the same workspace", () => { + const { result } = renderEnabled(WORKSPACE_A); + + act(() => { + window.localStorage.setItem(`${STORAGE_KEY}:${WORKSPACE_A}`, "false"); + window.dispatchEvent(new Event(SYNC_EVENT)); + }); + + expect(result.current.enabled).toBe(false); + }); +}); + +describe("readIntegrationEnabled", () => { + beforeEach(() => { + localStorageMock.clear(); + resetIntegrationEnabledMigrations(); + }); + + it("keys the value by workspace, falling back to the install-wide value", () => { + window.localStorage.setItem(STORAGE_KEY, "false"); + window.localStorage.setItem(`${STORAGE_KEY}:${WORKSPACE_A}`, "true"); + + expect(readIntegrationEnabled(STORAGE_KEY, WORKSPACE_A)).toBe(true); + expect(readIntegrationEnabled(STORAGE_KEY, WORKSPACE_B)).toBe(false); + expect(integrationEnabledStorageKey(STORAGE_KEY, WORKSPACE_A)).toBe( + `${STORAGE_KEY}:${WORKSPACE_A}`, + ); + expect(integrationEnabledStorageKey(STORAGE_KEY, null)).toBe(STORAGE_KEY); + }); +}); + +describe("useIntegrationEnabledReader", () => { + beforeEach(() => { + localStorageMock.clear(); + resetIntegrationEnabledMigrations(); + }); + + it("reads any workspace's toggle without a hook per workspace", () => { + window.localStorage.setItem(`${STORAGE_KEY}:${WORKSPACE_A}`, "false"); + const { result } = renderHook(() => useIntegrationEnabledReader([SYNC_EVENT])); + + expect(result.current(KEYS, WORKSPACE_A)).toBe(false); + expect(result.current(KEYS, WORKSPACE_B)).toBe(true); + }); + + it("migrates a legacy key itself, with no `useIntegrationEnabled` mounted", () => { + // The settings tree is built on the reader alone, so if only the hook + // migrated, a legacy value would read as enabled there. + window.localStorage.setItem(legacyKey(WORKSPACE_A), "false"); + const { result } = renderHook(() => useIntegrationEnabledReader([SYNC_EVENT])); + + expect(result.current(KEYS, WORKSPACE_A)).toBe(false); + expect(window.localStorage.getItem(`${STORAGE_KEY}:${WORKSPACE_A}`)).toBe("false"); + expect(window.localStorage.getItem(legacyKey(WORKSPACE_A))).toBeNull(); + }); + + it("takes a new identity on a toggle, so memoized consumers recompute", () => { + const { result } = renderHook(() => useIntegrationEnabledReader([SYNC_EVENT])); + const before = result.current; + + act(() => { + window.localStorage.setItem(`${STORAGE_KEY}:${WORKSPACE_A}`, "false"); + window.dispatchEvent(new Event(SYNC_EVENT)); + }); + + expect(result.current).not.toBe(before); + expect(result.current(KEYS, WORKSPACE_A)).toBe(false); + }); + + it("also reacts to another tab's write", () => { + const { result } = renderHook(() => useIntegrationEnabledReader([SYNC_EVENT])); + const before = result.current; + + act(() => { + window.localStorage.setItem(`${STORAGE_KEY}:${WORKSPACE_A}`, "false"); + window.dispatchEvent( + new StorageEvent("storage", { + key: `${STORAGE_KEY}:${WORKSPACE_A}`, + newValue: "false", + }), + ); + }); + + expect(result.current).not.toBe(before); + }); +}); diff --git a/apps/web/hooks/domains/integrations/use-integration-enabled.ts b/apps/web/hooks/domains/integrations/use-integration-enabled.ts index 22b5b0c4d0..513beb5d7a 100644 --- a/apps/web/hooks/domains/integrations/use-integration-enabled.ts +++ b/apps/web/hooks/domains/integrations/use-integration-enabled.ts @@ -1,52 +1,99 @@ "use client"; -import { useCallback, useMemo, useSyncExternalStore } from "react"; +import { useCallback, useEffect, useMemo, useState, useSyncExternalStore } from "react"; -// useIntegrationEnabled is the install-wide on/off toggle every third-party +// useIntegrationEnabled is the per-workspace on/off toggle every third-party // integration UI (jira, linear, future) needs: a localStorage-backed boolean // that defaults to true, syncs across tabs via the `storage` event, and within // a tab via a custom event the integration provides. // -// Connection state is install-wide (one Jira account, one Linear account), so -// the toggle is too. The legacy per-workspace key is migrated transparently on -// first read. +// The state is scoped to a workspace because everything else about an +// integration already is: credentials, health, and the settings page the +// slider lives on all hang off `/settings/workspaces//integrations`. +// Disabling GitHub because one workspace doesn't use it must not silence it in +// the workspaces that do. +// +// The value lives at `:`. `storageKey` itself holds +// the pre-scoping install-wide value and is read as the seed for a workspace +// that has never been toggled, so upgrading does not silently re-enable an +// integration the user had turned off. // // The signature takes plain string parameters rather than an options object so // useSyncExternalStore's getSnapshot stays referentially stable across // renders — passing an inline `{...}` would give getSnapshot a new identity // every render and re-run the migration scan on each pass. -// migrateLegacyKey is idempotent: the first call moves any leftover -// per-workspace value onto storageKey and removes the legacy entries; -// subsequent calls are no-ops because there are no legacy keys left to scan. -// Cheap enough to run on every snapshot read (a localStorage iteration over -// kandev:* keys), so we don't need to memoize across re-renders. -function migrateLegacyKey(storageKey: string, legacyKeyPrefix: string): void { - if (typeof window === "undefined") return; +const LEGACY_KEY_SUFFIX = ":v1"; + +// Prefixes already scanned this session. The scan is a full localStorage +// iteration and `getSnapshot` runs on every render, so without this a browser +// that has no legacy keys (every install since Aug 2026) would re-scan forever. +const scannedLegacyPrefixes = new Set(); + +/** Test-only: forget which prefixes were scanned, so a case can re-run one. */ +export function resetIntegrationEnabledMigrations(): void { + scannedLegacyPrefixes.clear(); +} + +// The pre-`v1` keys were `kandev::enabled::v1` — already one +// per workspace. Each is restored to that workspace's own key rather than +// folded into the install-wide one: folding is what let a single workspace's +// preference answer for every other workspace, which is the bug this scoping +// exists to fix. An id-less or unrecognized key is left alone rather than +// guessed at. +// +// Today's keys (`:`) share the legacy prefix, so they +// are skipped explicitly — otherwise the migration would eat its own output. +function migrateLegacyKeys(storageKey: string, legacyKeyPrefix: string): void { + if (typeof window === "undefined" || scannedLegacyPrefixes.has(storageKey)) return; + scannedLegacyPrefixes.add(storageKey); try { - if (window.localStorage.getItem(storageKey) !== null) return; - let surviving: string | null = null; - const stale: string[] = []; + // Collected before writing: mutating localStorage mid-scan shifts the + // indices `key(i)` walks. + const legacy: Array<{ key: string; workspaceId: string }> = []; for (let i = 0; i < window.localStorage.length; i++) { const key = window.localStorage.key(i); - if (!key || !key.startsWith(legacyKeyPrefix) || key === storageKey) continue; - stale.push(key); + if (!key || !key.startsWith(legacyKeyPrefix) || key.startsWith(storageKey)) continue; + const scoped = key.slice(legacyKeyPrefix.length); + if (!scoped.endsWith(LEGACY_KEY_SUFFIX)) continue; + const workspaceId = scoped.slice(0, -LEGACY_KEY_SUFFIX.length); + if (workspaceId) legacy.push({ key, workspaceId }); + } + for (const { key, workspaceId } of legacy) { const value = window.localStorage.getItem(key); - if (value !== null && surviving === null) surviving = value; + const target = integrationEnabledStorageKey(storageKey, workspaceId); + // A value the user has since set for that workspace wins over the + // migration; the legacy entry goes either way. + if (value !== null && window.localStorage.getItem(target) === null) { + window.localStorage.setItem(target, value); + } + window.localStorage.removeItem(key); } - if (surviving !== null) { - window.localStorage.setItem(storageKey, surviving); - } - for (const k of stale) window.localStorage.removeItem(k); } catch { // Quota / private mode — fall through; the toggle just defaults to on. } } -function readEnabled(storageKey: string): boolean { +/** Where a workspace's value lives; the bare key when no workspace is known. */ +export function integrationEnabledStorageKey( + storageKey: string, + workspaceId?: string | null, +): string { + return workspaceId ? `${storageKey}:${workspaceId}` : storageKey; +} + +/** + * A workspace's toggle, defaulting to the pre-scoping install-wide value and + * then to on. Pure read: no subscription, for callers that must sample several + * workspaces at once (see `useIntegrationEnabledReader`). + */ +export function readIntegrationEnabled(storageKey: string, workspaceId?: string | null): boolean { if (typeof window === "undefined") return true; try { - const raw = window.localStorage.getItem(storageKey); + const scoped = window.localStorage.getItem( + integrationEnabledStorageKey(storageKey, workspaceId), + ); + const raw = scoped ?? window.localStorage.getItem(storageKey); if (raw === null) return true; return raw !== "false"; } catch { @@ -58,6 +105,7 @@ export function useIntegrationEnabled( storageKey: string, legacyKeyPrefix: string, syncEvent: string, + workspaceId?: string | null, ) { // useSyncExternalStore reads localStorage on every render, but the snapshot // is referentially stable (a boolean) so React only re-renders when the @@ -78,9 +126,9 @@ export function useIntegrationEnabled( ); const getSnapshot = useCallback(() => { - migrateLegacyKey(storageKey, legacyKeyPrefix); - return readEnabled(storageKey); - }, [storageKey, legacyKeyPrefix]); + migrateLegacyKeys(storageKey, legacyKeyPrefix); + return readIntegrationEnabled(storageKey, workspaceId); + }, [storageKey, legacyKeyPrefix, workspaceId]); const getServerSnapshot = useCallback(() => true, []); @@ -89,14 +137,15 @@ export function useIntegrationEnabled( const setEnabled = useCallback( (next: boolean) => { if (typeof window === "undefined") return; + const key = integrationEnabledStorageKey(storageKey, workspaceId); try { - window.localStorage.setItem(storageKey, String(next)); + window.localStorage.setItem(key, String(next)); } catch (error) { - throw new Error(`Failed to persist ${storageKey}`, { cause: error }); + throw new Error(`Failed to persist ${key}`, { cause: error }); } window.dispatchEvent(new Event(syncEvent)); }, - [storageKey, syncEvent], + [storageKey, syncEvent, workspaceId], ); // `loaded` is always true with useSyncExternalStore — the snapshot is read @@ -104,3 +153,60 @@ export function useIntegrationEnabled( // callers (which gated effects on `loaded`) don't need to change. return { enabled, setEnabled, loaded: true }; } + +/** The storage identity of one integration's toggle, as the reader needs it. */ +export type IntegrationEnabledKeyPair = { + storageKey: string; + legacyKeyPrefix: string; +}; + +/** Reads one (integration, workspace) pair, migrating legacy keys first. */ +export type IntegrationEnabledReader = ( + keys: IntegrationEnabledKeyPair, + workspaceId?: string | null, +) => boolean; + +function makeIntegrationEnabledReader(): IntegrationEnabledReader { + return ({ storageKey, legacyKeyPrefix }, workspaceId) => { + // The hook form migrates in its own snapshot read; a surface built only on + // the reader (the settings tree lists every workspace and mounts no + // `useXEnabled`) would otherwise never see a legacy value at all. + migrateLegacyKeys(storageKey, legacyKeyPrefix); + return readIntegrationEnabled(storageKey, workspaceId); + }; +} + +/** + * A `readIntegrationEnabled` bound to a live subscription, for surfaces that + * read many (integration, workspace) pairs at once — the settings menu tree + * lists every workspace, and rules of hooks forbid calling `useXEnabled` in a + * loop over them. + * + * The reader is held in state and replaced on every toggle: its identity *is* + * the change signal, which is what re-runs a consumer's memoized derivation. + * Returning a fresh closure each render would work too, but it would defeat the + * memo the settings menu builds its whole branch forest behind. Pass a + * module-level constant for `syncEvents`. + */ +export function useIntegrationEnabledReader( + syncEvents: readonly string[], +): IntegrationEnabledReader { + const [read, setRead] = useState(makeIntegrationEnabledReader); + // Joined rather than spread so the effect keys off the event names' contents, + // not the array's identity. + const eventKey = syncEvents.join("|"); + + useEffect(() => { + if (typeof window === "undefined") return; + const bump = () => setRead(() => makeIntegrationEnabledReader()); + const events = eventKey ? eventKey.split("|") : []; + window.addEventListener("storage", bump); + for (const event of events) window.addEventListener(event, bump); + return () => { + window.removeEventListener("storage", bump); + for (const event of events) window.removeEventListener(event, bump); + }; + }, [eventKey]); + + return read; +} diff --git a/apps/web/hooks/domains/jira/use-jira-availability.test.ts b/apps/web/hooks/domains/jira/use-jira-availability.test.ts index 0e7886437f..0730d13ef8 100644 --- a/apps/web/hooks/domains/jira/use-jira-availability.test.ts +++ b/apps/web/hooks/domains/jira/use-jira-availability.test.ts @@ -66,8 +66,9 @@ describe("useJiraAvailable", () => { window.localStorage.setItem("kandev:jira:enabled:v1", "false"); getJiraConfigMock.mockResolvedValue(makeConfig({ hasSecret: true, lastOk: true })); const { result } = renderHook(() => useJiraAvailable()); - // The toggle is install-wide now; an off-toggle keeps `enabled` at false - // while the auth probe still runs in the background. + // No workspace is passed here, so the toggle read is the unscoped one; an + // off-toggle keeps `enabled` at false while the auth probe still runs in + // the background. await waitFor(() => expect(result.current).toBe(false)); }); diff --git a/apps/web/hooks/domains/jira/use-jira-availability.ts b/apps/web/hooks/domains/jira/use-jira-availability.ts index 564842bf31..ea88fc599b 100644 --- a/apps/web/hooks/domains/jira/use-jira-availability.ts +++ b/apps/web/hooks/domains/jira/use-jira-availability.ts @@ -22,7 +22,7 @@ export function useJiraAvailable(workspaceId?: string | null): boolean { [workspaceId], ); return useIntegrationAvailable({ - useEnabled: useJiraEnabled, + enabledState: useJiraEnabled(workspaceId), fetchConfig, }); } diff --git a/apps/web/hooks/domains/jira/use-jira-enabled.ts b/apps/web/hooks/domains/jira/use-jira-enabled.ts index 9c73b8371e..592ae4dc4e 100644 --- a/apps/web/hooks/domains/jira/use-jira-enabled.ts +++ b/apps/web/hooks/domains/jira/use-jira-enabled.ts @@ -1,11 +1,18 @@ "use client"; +import { INTEGRATION_ENABLED_KEYS } from "@/lib/integrations/integration-enabled-keys"; import { useIntegrationEnabled } from "../integrations/use-integration-enabled"; -const STORAGE_KEY = "kandev:jira:enabled:v1"; -const LEGACY_KEY_PREFIX = "kandev:jira:enabled:"; -const SYNC_EVENT = "kandev:jira:enabled-changed"; +const { storageKey, legacyKeyPrefix, syncEvent } = INTEGRATION_ENABLED_KEYS["jira"]; -export function useJiraEnabled() { - return useIntegrationEnabled(STORAGE_KEY, LEGACY_KEY_PREFIX, SYNC_EVENT); +/** + * Per-workspace enable/disable state for the Jira integration. + * + * Backed by `localStorage` (key `kandev:jira:enabled:v1:`), synced across + * browser tabs and across the own-settings-page and index-page sliders. + * Defaults to `true` when no value has ever been persisted for the workspace. + * Omitting `workspaceId` reads the pre-scoping install-wide value. + */ +export function useJiraEnabled(workspaceId?: string | null) { + return useIntegrationEnabled(storageKey, legacyKeyPrefix, syncEvent, workspaceId); } diff --git a/apps/web/hooks/domains/linear/use-linear-availability.ts b/apps/web/hooks/domains/linear/use-linear-availability.ts index 162745154a..ea30740f1d 100644 --- a/apps/web/hooks/domains/linear/use-linear-availability.ts +++ b/apps/web/hooks/domains/linear/use-linear-availability.ts @@ -22,7 +22,7 @@ export function useLinearAvailable(workspaceId?: string | null): boolean { [workspaceId], ); return useIntegrationAvailable({ - useEnabled: useLinearEnabled, + enabledState: useLinearEnabled(workspaceId), fetchConfig, }); } diff --git a/apps/web/hooks/domains/linear/use-linear-enabled.test.ts b/apps/web/hooks/domains/linear/use-linear-enabled.test.ts index 691b6b9caa..ddfbb5570e 100644 --- a/apps/web/hooks/domains/linear/use-linear-enabled.test.ts +++ b/apps/web/hooks/domains/linear/use-linear-enabled.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import { act, renderHook, waitFor } from "@testing-library/react"; import { useLinearEnabled } from "./use-linear-enabled"; +import { resetIntegrationEnabledMigrations } from "../integrations/use-integration-enabled"; const STORAGE_KEY = "kandev:linear:enabled:v1"; @@ -36,6 +37,7 @@ Object.defineProperty(window, "localStorage", { describe("useLinearEnabled", () => { beforeEach(() => { localStorageMock.clear(); + resetIntegrationEnabledMigrations(); }); afterEach(() => { localStorageMock.clear(); @@ -74,17 +76,20 @@ describe("useLinearEnabled", () => { expect(window.localStorage.getItem(STORAGE_KEY)).toBe("false"); }); - it("migrates a legacy per-workspace key into the new install-wide key on first read", async () => { - // Single legacy entry so the migration outcome is deterministic — with - // multiple, the "first one we encounter" depends on localStorage iteration - // order, which the test shouldn't depend on. - window.localStorage.setItem("kandev:linear:enabled:ws-1:v1", "false"); - const { result } = renderHook(() => useLinearEnabled()); - await waitFor(() => expect(result.current.loaded).toBe(true)); + it("restores a legacy per-workspace key to that workspace's own key", async () => { + // The pre-scoping keys were `kandev:linear:enabled::v1`, one per + // workspace, so each is restored to the workspace it came from rather than + // folded into a single install-wide value. + window.localStorage.setItem("kandev:linear:enabled:ws-123:v1", "false"); + const { result } = renderHook(() => useLinearEnabled("ws-123")); + + await waitFor(() => expect(result.current.loaded).toBe(true)); expect(result.current.enabled).toBe(false); - expect(window.localStorage.getItem(STORAGE_KEY)).toBe("false"); - expect(window.localStorage.getItem("kandev:linear:enabled:ws-1:v1")).toBeNull(); + expect(window.localStorage.getItem(`${STORAGE_KEY}:ws-123`)).toBe("false"); + expect(window.localStorage.getItem("kandev:linear:enabled:ws-123:v1")).toBeNull(); + // Another workspace is untouched by that migration. + expect(renderHook(() => useLinearEnabled("ws-other")).result.current.enabled).toBe(true); }); it("propagates updates dispatched via the kandev:linear:enabled-changed event", async () => { diff --git a/apps/web/hooks/domains/linear/use-linear-enabled.ts b/apps/web/hooks/domains/linear/use-linear-enabled.ts index e6e4e2ccb0..a96d893db4 100644 --- a/apps/web/hooks/domains/linear/use-linear-enabled.ts +++ b/apps/web/hooks/domains/linear/use-linear-enabled.ts @@ -1,11 +1,18 @@ "use client"; +import { INTEGRATION_ENABLED_KEYS } from "@/lib/integrations/integration-enabled-keys"; import { useIntegrationEnabled } from "../integrations/use-integration-enabled"; -const STORAGE_KEY = "kandev:linear:enabled:v1"; -const LEGACY_KEY_PREFIX = "kandev:linear:enabled:"; -const SYNC_EVENT = "kandev:linear:enabled-changed"; +const { storageKey, legacyKeyPrefix, syncEvent } = INTEGRATION_ENABLED_KEYS["linear"]; -export function useLinearEnabled() { - return useIntegrationEnabled(STORAGE_KEY, LEGACY_KEY_PREFIX, SYNC_EVENT); +/** + * Per-workspace enable/disable state for the Linear integration. + * + * Backed by `localStorage` (key `kandev:linear:enabled:v1:`), synced across + * browser tabs and across the own-settings-page and index-page sliders. + * Defaults to `true` when no value has ever been persisted for the workspace. + * Omitting `workspaceId` reads the pre-scoping install-wide value. + */ +export function useLinearEnabled(workspaceId?: string | null) { + return useIntegrationEnabled(storageKey, legacyKeyPrefix, syncEvent, workspaceId); } diff --git a/apps/web/hooks/domains/sentry/use-sentry-availability.ts b/apps/web/hooks/domains/sentry/use-sentry-availability.ts index 34ef288da8..f4f5f0d010 100644 --- a/apps/web/hooks/domains/sentry/use-sentry-availability.ts +++ b/apps/web/hooks/domains/sentry/use-sentry-availability.ts @@ -35,7 +35,7 @@ export type SentryAvailability = { // surfaces and settings banner consume. Fetches are request-versioned so a slow // response for a previous workspace can't clobber a newer one. export function useSentryInstances(workspaceId?: string | null): SentryAvailability { - const { enabled, loaded } = useSentryEnabled(); + const { enabled, loaded } = useSentryEnabled(workspaceId); const active = loaded && enabled && !!workspaceId; const [instances, setInstances] = useState([]); const [loading, setLoading] = useState(true); diff --git a/apps/web/hooks/domains/sentry/use-sentry-enabled.ts b/apps/web/hooks/domains/sentry/use-sentry-enabled.ts index 89b45129e8..61e47fcaeb 100644 --- a/apps/web/hooks/domains/sentry/use-sentry-enabled.ts +++ b/apps/web/hooks/domains/sentry/use-sentry-enabled.ts @@ -1,11 +1,18 @@ "use client"; +import { INTEGRATION_ENABLED_KEYS } from "@/lib/integrations/integration-enabled-keys"; import { useIntegrationEnabled } from "../integrations/use-integration-enabled"; -const STORAGE_KEY = "kandev:sentry:enabled:v1"; -const LEGACY_KEY_PREFIX = "kandev:sentry:enabled:"; -const SYNC_EVENT = "kandev:sentry:enabled-changed"; +const { storageKey, legacyKeyPrefix, syncEvent } = INTEGRATION_ENABLED_KEYS["sentry"]; -export function useSentryEnabled() { - return useIntegrationEnabled(STORAGE_KEY, LEGACY_KEY_PREFIX, SYNC_EVENT); +/** + * Per-workspace enable/disable state for the Sentry integration. + * + * Backed by `localStorage` (key `kandev:sentry:enabled:v1:`), synced across + * browser tabs and across the own-settings-page and index-page sliders. + * Defaults to `true` when no value has ever been persisted for the workspace. + * Omitting `workspaceId` reads the pre-scoping install-wide value. + */ +export function useSentryEnabled(workspaceId?: string | null) { + return useIntegrationEnabled(storageKey, legacyKeyPrefix, syncEvent, workspaceId); } diff --git a/apps/web/hooks/use-nav-availability.test.ts b/apps/web/hooks/use-nav-availability.test.ts index 2da345b10c..0c060879f2 100644 --- a/apps/web/hooks/use-nav-availability.test.ts +++ b/apps/web/hooks/use-nav-availability.test.ts @@ -11,7 +11,7 @@ const mocks = vi.hoisted(() => { state: { workspaces: { activeId: workspaceId as string | null, - items: [{ id: workspaceId }], + items: [{ id: workspaceId }] as Array<{ id: string; created_at?: string }>, }, }, azureDevOpsAvailable: vi.fn(() => false), @@ -115,6 +115,9 @@ describe("getGitHubIntegrationStatus", () => { }); }); +/** A workspace that is not the (stale) active one. */ +const SURVIVING_WORKSPACE_ID = "workspace-2"; + const NAV_GATED_KEYS: AvailabilityKey[] = ["azure-devops", "github", "gitlab", "jira", "linear"]; const ENABLED_MOCK_BY_KEY = { @@ -170,14 +173,43 @@ describe("useNavAvailability", () => { expect(mocks.azureDevOpsAvailable).toHaveBeenCalledWith(mocks.workspaceId); }); + it("reads each enable toggle for the active workspace, not install-wide", () => { + // The toggles are per workspace: a workspace with GitHub turned off must + // not hide it from the nav while another workspace is active. + renderHook(() => useNavAvailability()); + + for (const key of NAV_GATED_KEYS) { + expect(ENABLED_MOCK_BY_KEY[key]).toHaveBeenCalledWith(mocks.workspaceId); + } + }); + it("falls back to default workspace resolution for a stale active id", () => { - mocks.state.workspaces.items = [{ id: "workspace-2" }]; + mocks.state.workspaces.items = [{ id: SURVIVING_WORKSPACE_ID }]; renderHook(() => useNavAvailability()); expect(mocks.jiraAuthed).toHaveBeenCalledWith(null); expect(mocks.linearAuthed).toHaveBeenCalledWith(null); expect(mocks.azureDevOpsAvailable).toHaveBeenCalledWith(null); + // The toggles are browser-local, so null would read the unscoped key while + // the probes above answer for whichever workspace the backend resolved. + // They follow the backend's tie-breaker (oldest workspace) instead. + for (const key of NAV_GATED_KEYS) { + expect(ENABLED_MOCK_BY_KEY[key]).toHaveBeenCalledWith(SURVIVING_WORKSPACE_ID); + } + }); + + it("reads the oldest workspace's toggles when the active id is stale", () => { + mocks.state.workspaces.items = [ + { id: "workspace-3", created_at: "2026-02-01T00:00:00Z" }, + { id: SURVIVING_WORKSPACE_ID, created_at: "2026-01-01T00:00:00Z" }, + ]; + + renderHook(() => useNavAvailability()); + + for (const key of NAV_GATED_KEYS) { + expect(ENABLED_MOCK_BY_KEY[key]).toHaveBeenCalledWith(SURVIVING_WORKSPACE_ID); + } }); describe.each(NAV_GATED_KEYS)("decoupling enabled from nav visibility for %s", (key) => { diff --git a/apps/web/hooks/use-nav-availability.ts b/apps/web/hooks/use-nav-availability.ts index 7306733cdb..011d116820 100644 --- a/apps/web/hooks/use-nav-availability.ts +++ b/apps/web/hooks/use-nav-availability.ts @@ -15,6 +15,19 @@ import { useHideDisabledIntegrationsInNav } from "@/hooks/domains/integrations/u import type { AvailabilityMap } from "@/lib/navigation/types"; import type { GitHubStatus } from "@/lib/types/github"; +/** + * The workspace the backend resolves when a request names none: the oldest by + * creation time. Ties (or an empty list) give null, which reads the unscoped + * toggle exactly as before. + */ +function oldestWorkspace(items: ReadonlyArray<{ id: string; created_at?: string }>): string | null { + let oldest: { id: string; created_at?: string } | null = null; + for (const item of items) { + if (!oldest || (item.created_at ?? "") < (oldest.created_at ?? "")) oldest = item; + } + return oldest?.id ?? null; +} + /** Human-readable status label for a not-yet-connected integration destination. */ function getStatusLabel(loading: boolean | undefined): string { return loading ? "Checking" : "Setup"; @@ -77,6 +90,14 @@ export function useNavAvailability(): AvailabilityMap { // when another workspace is configured. Fall back to null so the backend's // default-workspace resolution applies instead. const scopedWorkspaceId = activeWorkspaceExists ? activeWorkspaceId : null; + // The toggles are browser-local, so a null id has no backend to resolve it: + // it would read the unscoped key while the probes above answer for whichever + // workspace the backend picked, and the two could then disagree about the + // same integration. Mirror the backend's tie-breaker instead + // (`workspacescope.ResolveMigrationTarget`: oldest workspace by creation), + // which with the usual single surviving workspace is exactly it. + const oldestWorkspaceId = useAppStore((s) => oldestWorkspace(s.workspaces.items)); + const toggleWorkspaceId = scopedWorkspaceId ?? oldestWorkspaceId; const { status, loading } = useGitHubStatus(); const gitlabConfigured = useGitLabAvailable(); const jiraConfigured = useJiraAuthed(scopedWorkspaceId); @@ -84,11 +105,13 @@ export function useNavAvailability(): AvailabilityMap { const azureDevOpsConfigured = useAzureDevOpsAvailable(scopedWorkspaceId); const githubConfigured = getGitHubIntegrationStatus(status, loading).ready; - const { enabled: azureDevOpsEnabled } = useAzureDevOpsEnabled(); - const { enabled: githubEnabled } = useGitHubEnabled(); - const { enabled: gitlabEnabled } = useGitLabEnabled(); - const { enabled: jiraEnabled } = useJiraEnabled(); - const { enabled: linearEnabled } = useLinearEnabled(); + // The toggles are per-workspace too: a workspace that has GitHub turned off + // must not hide it from the nav while another workspace is active. + const { enabled: azureDevOpsEnabled } = useAzureDevOpsEnabled(toggleWorkspaceId); + const { enabled: githubEnabled } = useGitHubEnabled(toggleWorkspaceId); + const { enabled: gitlabEnabled } = useGitLabEnabled(toggleWorkspaceId); + const { enabled: jiraEnabled } = useJiraEnabled(toggleWorkspaceId); + const { enabled: linearEnabled } = useLinearEnabled(toggleWorkspaceId); const { hideDisabled } = useHideDisabledIntegrationsInNav(); const visible = (configured: boolean, enabled: boolean) => diff --git a/apps/web/lib/integrations/integration-enabled-keys.ts b/apps/web/lib/integrations/integration-enabled-keys.ts new file mode 100644 index 0000000000..379a3cd96d --- /dev/null +++ b/apps/web/lib/integrations/integration-enabled-keys.ts @@ -0,0 +1,42 @@ +import { WORKSPACE_INTEGRATIONS } from "@/lib/settings-discovery/catalog/integrations"; + +export type IntegrationSlug = (typeof WORKSPACE_INTEGRATIONS)[number][0]; + +export type IntegrationEnabledKeys = { + /** + * Base `localStorage` key. The per-workspace value lives at + * `:`; the base key itself holds the pre-scoping + * install-wide value, which now seeds a workspace that has never been + * toggled. + */ + storageKey: string; + /** Pre-`v1` per-workspace keys, folded into `storageKey` on first read. */ + legacyKeyPrefix: string; + /** Same-tab broadcast event, so both sliders re-render on a toggle. */ + syncEvent: string; +}; + +function keysFor(slug: string): IntegrationEnabledKeys { + return { + storageKey: `kandev:${slug}:enabled:v1`, + legacyKeyPrefix: `kandev:${slug}:enabled:`, + syncEvent: `kandev:${slug}:enabled-changed`, + }; +} + +/** + * Every integration's enable-toggle storage identity in one place. + * + * The per-integration `useXEnabled` hooks read their own entry, so the naming + * scheme is declared once. Surfaces that must read *several* workspaces' + * toggles at once (the settings menu tree) cannot call those hooks in a loop + * and read through this catalog with `readIntegrationEnabled` instead. + */ +export const INTEGRATION_ENABLED_KEYS = Object.fromEntries( + WORKSPACE_INTEGRATIONS.map(([slug]) => [slug, keysFor(slug)]), +) as Record; + +/** Every integration's same-tab toggle event, for multi-integration readers. */ +export const INTEGRATION_ENABLED_SYNC_EVENTS: readonly string[] = WORKSPACE_INTEGRATIONS.map( + ([slug]) => INTEGRATION_ENABLED_KEYS[slug].syncEvent, +); diff --git a/docs/specs/integrations/enable-disable-toggle.md b/docs/specs/integrations/enable-disable-toggle.md index 5d456fa985..3b4e1635d3 100644 --- a/docs/specs/integrations/enable-disable-toggle.md +++ b/docs/specs/integrations/enable-disable-toggle.md @@ -19,21 +19,34 @@ disabled-but-still-configured integration continues to clutter the left panel navigation. (Slack was retired from the core app to an external plugin after this spec was written and is out of scope.) +"Because a workspace doesn't use it" was one of the reasons above, but the +toggle first shipped install-wide: turning GitHub off for one workspace turned +it off for every workspace, while the credentials it gates stay per workspace. +That is corrected here, and the requirements below now describe the +per-workspace toggle. + ## What - Every integration (Azure DevOps, GitHub, GitLab, Jira, Linear, Sentry) SHALL expose an enable/disable slider on its own settings page (`/settings/integrations/`, and the per-workspace equivalent - `/settings/workspace//integrations/`). Jira, Linear and Sentry + `/settings/workspaces//integrations/`). Jira, Linear and Sentry already have this; Azure DevOps, GitHub and GitLab gain it. - The same slider SHALL also appear on each integration's row/card on the integrations index page (`/settings/integrations` and its per-workspace equivalent), so a user can enable or disable any integration without opening its detail page. -- Toggling the slider in either location SHALL keep both locations in sync - (same underlying per-integration enabled state, install-wide — not - per-workspace — consistent with the existing Jira/Linear/Sentry - toggle). +- The enabled state SHALL be scoped to a workspace. Everything else about an + integration already is (credentials, health, and the settings page the + slider lives on, `/settings/workspaces//integrations/`), so + disabling an integration in one workspace MUST NOT disable it in any other. + Every surface that reads the toggle reads it for the workspace it is + showing: the sliders for the workspace whose settings page they are on, the + left-panel nav and its "hide disabled" filter for the active workspace, and + the Settings menu tree for each workspace it lists. +- Toggling the slider in either location SHALL keep both locations in sync for + that workspace (same underlying per-integration, per-workspace enabled + state). - The enabled state for Azure DevOps, GitHub and GitLab is purely a presentation/navigation-visibility switch: it MUST NOT change whether their existing PR/work-item/board/MR features function. This mirrors that those @@ -62,7 +75,7 @@ this spec was written and is out of scope.) left-panel nav visibility. - All new/changed toggles persist and sync the same way the existing Jira/Linear/Sentry "enabled" toggle does: a `localStorage`-backed, - install-wide boolean, synced across browser tabs, with a manual-save + per-workspace boolean, synced across browser tabs, with a manual-save affordance consistent with the rest of the Settings UI (`data-settings-dirty` + the shared save bar) where the control lives on a settings page. @@ -74,13 +87,24 @@ in `hooks/domains/integrations/use-integration-enabled.ts`: | Key | Type | Default | Notes | |---|---|---|---| -| `kandev:azure-devops:enabled:v1` | boolean | `true` | New. Mirrors `kandev:jira:enabled:v1`. | -| `kandev:github:enabled:v1` | boolean | `true` | New. | -| `kandev:gitlab:enabled:v1` | boolean | `true` | New. | -| `kandev:integrations:hideDisabledInNav:v1` | boolean | `false` | New. Not per-integration; one flag for the whole nav-filtering behavior. | +| `kandev::enabled:v1:` | boolean | `true` | One per (integration, workspace), for all six slugs. | +| `kandev:integrations:hideDisabledInNav:v1` | boolean | `false` | Not per-integration and not per-workspace; one flag for the whole nav-filtering behavior. | -Existing keys (`kandev:jira:enabled:v1`, `kandev:linear:enabled:v1`, -`kandev:sentry:enabled:v1`) are unchanged. +The unsuffixed `kandev::enabled:v1` keys hold the value written while the +toggle was install-wide. They are never written again, and are read as the +default for a workspace that has never been toggled, so upgrading does not +silently re-enable an integration a user had turned off. + +Keys from before that install-wide period (`kandev::enabled::v1`) +were themselves per workspace. Each SHALL be restored to that workspace's own +key, never folded into the unsuffixed one: folding would let a single +workspace's stored preference answer for every other workspace, which is the +behavior this scoping exists to remove. A workspace that already has a value +keeps it, the legacy entry is then dropped either way, and a legacy key with no +recoverable workspace id is left untouched rather than guessed at. The scan +skips the current suffixed keys (they share the prefix) and runs at most once +per integration per page load, so a browser holding no legacy keys pays for one +pass rather than one per render. ## API surface @@ -89,13 +113,25 @@ No backend/HTTP/WS changes. This is entirely a frontend feature. Frontend primitives (new): - `hooks/domains/azure-devops/use-azure-devops-enabled.ts` — exports - `useAzureDevOpsEnabled()`, a one-line wrapper over - `useIntegrationEnabled(storageKey, legacyKeyPrefix, syncEvent)`, mirroring - `hooks/domains/jira/use-jira-enabled.ts`. No legacy key prefix existed - before, so the legacy-migration argument is inert (no prior keys to + `useAzureDevOpsEnabled(workspaceId?)`, a one-line wrapper over + `useIntegrationEnabled(storageKey, legacyKeyPrefix, syncEvent, workspaceId)`, + mirroring `hooks/domains/jira/use-jira-enabled.ts`. No legacy key prefix + existed before, so the legacy-migration argument is inert (no prior keys to migrate) but kept for signature symmetry. - `hooks/domains/github/use-github-enabled.ts` — same shape, for GitHub. - `hooks/domains/gitlab/use-gitlab-enabled.ts` — same shape, for GitLab. +- `lib/integrations/integration-enabled-keys.ts` — the per-slug storage + key / legacy prefix / sync event catalog, so the naming scheme is declared + once and surfaces that must read several workspaces' toggles at a time have + something to read through. +- `hooks/domains/integrations/use-integration-enabled.ts` also exports + `readIntegrationEnabled` (an unsubscribed read for one workspace) and + `useIntegrationEnabledReader` (the same read bound to a live subscription). + The Settings menu tree lists every workspace and rules of hooks forbid + calling `useXEnabled` per workspace, so it uses the reader. +- `components/integrations/integration-enabled-control-props.ts` — the props + every slider takes. `workspaceId` is a required (though nullable) prop so a + render site that forgets to name its workspace fails to compile. - `hooks/domains/integrations/use-hide-disabled-integrations-in-nav.ts` — exports `useHideDisabledIntegrationsInNav()` returning `{ hideDisabled: boolean; setHideDisabled: (next: boolean) => void }`, @@ -137,9 +173,9 @@ Modified: ## Permissions -No change. The toggle is a per-browser-profile, install-wide UI preference -with no authorization dimension (same as the existing Jira/Linear/Sentry -toggle). +No change. The toggle is a per-browser-profile UI preference with no +authorization dimension; scoping it to a workspace is a scoping change, not an +access-control one. ## Failure modes @@ -162,7 +198,8 @@ toggle). ## Persistence guarantees -State lives only in `localStorage`; it is not part of any backend restart or +State lives only in `localStorage`, keyed by workspace id; it is not part of +any backend restart or task-execution durability guarantee, matching the existing enabled-toggle convention. It survives a browser restart but not a `localStorage` clear, and is not synced across devices/browsers. @@ -196,6 +233,16 @@ is not synced across devices/browsers. one of the six integration rows/cards shows its own enable/disable slider reflecting that integration's current stored state, and toggling a slider does not navigate to that integration's detail page. +- **GIVEN** two workspaces and GitHub connected in both, **WHEN** the user + disables GitHub from workspace A's integrations page, **THEN** workspace B's + GitHub slider still reads "Enabled", and every surface gated on GitHub's + enabled state (nav filtering while "hide disabled" is on, the Settings menu + tree's per-workspace Integrations list) treats GitHub as enabled while B is + the workspace in view. +- **GIVEN** a user who had disabled an integration while the toggle was + install-wide, **WHEN** they upgrade to the per-workspace toggle, **THEN** + every workspace still reads it as disabled until one of them toggles it, + after which only that workspace's value changes. - **GIVEN** the integrations index page and the Azure DevOps settings page open in two browser tabs, **WHEN** the user disables Azure DevOps on the index page in tab A, **THEN** tab B's Azure DevOps settings-page slider