From 9dde803efb4e06b61b6ba73b3ba14bacd770aa23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 12 Nov 2026 13:48:08 +0100 Subject: [PATCH] Revert "fix(settings): list the Coach entry only while the Coach can answer" This reverts commit 0d41abcda. With the entry hidden, Settings, AI showed the stored Coach memory instead, and that card reads the conversation list: an account without any AI provider then made a Coach request on a page that should make none. Settings lists the Coach while its module is on, as before. --- .../coach-entry-follows-capability.test.tsx | 89 ------------------- src/components/settings/ai-section.tsx | 12 +-- src/components/settings/settings-shell.tsx | 7 +- 3 files changed, 9 insertions(+), 99 deletions(-) delete mode 100644 src/components/settings/__tests__/coach-entry-follows-capability.test.tsx diff --git a/src/components/settings/__tests__/coach-entry-follows-capability.test.tsx b/src/components/settings/__tests__/coach-entry-follows-capability.test.tsx deleted file mode 100644 index c30b49e8c..000000000 --- a/src/components/settings/__tests__/coach-entry-follows-capability.test.tsx +++ /dev/null @@ -1,89 +0,0 @@ -/** - * Settings lists the Coach entry only while the Coach can answer, like the - * sidebar and the bottom bar, and Settings → AI then keeps the stored Coach - * memory in reach. - * - * Both used to read the raw module map, which answers the operator's switch - * and Hide Coach but not a missing provider or missing consent: the settings - * nav kept a Coach entry the sidebar had already dropped. The two now read - * the same map (`useNavModules`), so the memory's own entry and the fallback - * under Settings → AI cannot both go. - * - * The shell's module filter runs after hydration only, so `useMounted` is - * mocked to the hydrated answer. - */ -import { beforeEach, describe, expect, it, vi } from "vitest"; -import { renderToStaticMarkup } from "react-dom/server"; - -const h = vi.hoisted(() => ({ coachAvailable: true })); - -vi.mock("next/navigation", () => ({ - usePathname: () => "/settings/ai", - useRouter: () => ({ replace: () => {}, push: () => {} }), -})); -vi.mock("@/hooks/use-mounted", () => ({ useMounted: () => true })); -vi.mock("@/hooks/use-auth", () => ({ - useAuth: () => ({ - user: { id: "u1", role: "USER", modules: { coach: true } }, - isAuthenticated: true, - isLoading: false, - refetch: vi.fn(), - }), -})); -vi.mock("@/hooks/use-ai-capability", () => ({ - useAiCapability: () => ({ - available: h.coachAvailable, - reason: h.coachAvailable ? null : "no_provider", - onDeviceAllowed: true, - }), -})); -// Settings → AI's cards have their own suites; only the memory slot is -// under test here. -vi.mock("@/components/ai/ai-consent-request", () => ({ - AiConsentRequest: () => null, -})); -vi.mock("@/components/settings/ai/ai-insights-card", () => ({ - AiInsightsCard: () => null, -})); -vi.mock("@/components/settings/ai/operator-off-notice", () => ({ - OperatorOffNotice: () => null, -})); -vi.mock("@/components/insights/ai-setup-hint", () => ({ - AiSetupHint: () => null, -})); -vi.mock("@/components/settings/coach-memory-section", () => ({ - StoredCoachMemory: () =>
, -})); - -import { I18nProvider } from "@/lib/i18n/context"; -import { AiSection } from "../ai-section"; -import { SettingsShell } from "../settings-shell"; - -function render(node: React.ReactNode) { - return renderToStaticMarkup( - {node}, - ); -} - -describe("the Coach entry in Settings", () => { - beforeEach(() => { - h.coachAvailable = true; - }); - - it("is listed while the Coach can answer, and the memory lives there", () => { - expect(render(body)).toContain( - 'href="/settings/coach"', - ); - expect(render()).not.toContain( - 'data-slot="stored-coach-memory"', - ); - }); - - it("goes when the Coach cannot answer, and Settings → AI keeps the memory", () => { - h.coachAvailable = false; - expect( - render(body), - ).not.toContain('href="/settings/coach"'); - expect(render()).toContain('data-slot="stored-coach-memory"'); - }); -}); diff --git a/src/components/settings/ai-section.tsx b/src/components/settings/ai-section.tsx index 30de702d7..57ae0dd16 100644 --- a/src/components/settings/ai-section.tsx +++ b/src/components/settings/ai-section.tsx @@ -25,12 +25,10 @@ import { StoredCoachMemory } from "@/components/settings/coach-memory-section"; import { AiSetupHint } from "@/components/insights/ai-setup-hint"; import { useAuth } from "@/hooks/use-auth"; import { useMounted } from "@/hooks/use-mounted"; -import { useNavModules } from "@/hooks/use-nav-modules"; import { isSurfaceVisible } from "@/lib/modules/surface"; export function AiSection() { - const { isAuthenticated } = useAuth(); - const navModules = useNavModules(); + const { isAuthenticated, user } = useAuth(); // v1.16.6 — the auth query can resolve before this boundary // hydrates; every `disabled={!isAuthenticated}` binding in the child // cards would then disagree with the SSR HTML (React #418). Gate the @@ -48,9 +46,11 @@ export function AiSection() { // nothing below reads as broken), and the one calm setup hint while no // provider is configured, or the consent question while the operator's // provider waits on it. Below it: what the Coach stored, while Settings → - // Coach is not listed (the settings nav reads the same map), so it can - // always be read and deleted. - const coachSettingsReachable = isSurfaceVisible("settings:coach", navModules); + // Coach is not reachable, so it can always be read and deleted. + const coachSettingsReachable = isSurfaceVisible( + "settings:coach", + user?.modules, + ); return (
diff --git a/src/components/settings/settings-shell.tsx b/src/components/settings/settings-shell.tsx index a2bcb5185..1ddee6b15 100644 --- a/src/components/settings/settings-shell.tsx +++ b/src/components/settings/settings-shell.tsx @@ -47,7 +47,6 @@ import { cn } from "@/lib/utils"; import { PageHeader } from "@/components/ui/page-header"; import { SUB_SHELL_GRID_FLOOR } from "@/components/layout/shell-metrics"; import { useAuth } from "@/hooks/use-auth"; -import { useNavModules } from "@/hooks/use-nav-modules"; import { useMounted } from "@/hooks/use-mounted"; import { useTranslations } from "@/lib/i18n/context"; import { isSurfaceVisible } from "@/lib/modules/surface"; @@ -367,8 +366,8 @@ export function SettingsShell({ const activeSlug = deriveActiveSlug(pathname, active); // v1.18.0 — per-submodule entries are listed only when their module - // is enabled, from `useNavModules()` as in the sidebar (Coach follows its - // AI capability; Settings → AI then lists the stored memory). Fail OPEN + // is enabled. Read from the resolved `useAuth().user.modules` map (the + // same map the Module hub, nav, and Insights pills gate off). Fail OPEN // (`!== false`): a missing key, or a not-yet-resolved `/me` payload, // reads as enabled so an entry never silently disappears. A section's // owner comes from the surface map (`settings:`); sections with no @@ -391,7 +390,7 @@ export function SettingsShell({ // effect (which the lint rule flags as a cascading render), and no risk of // the two passes disagreeing. const hydrated = useMounted(); - const modules = useNavModules(); + const modules = user?.modules; const activeRecord = user?.accountAccess?.active ?? null; const allVisibleSections = SETTINGS_SECTIONS.filter( (section) =>