From daaab5db0167e971fc9e767ae2dc5bf00fb246d6 Mon Sep 17 00:00:00 2001 From: os-zhuang Date: Thu, 25 Jun 2026 15:34:52 +0800 Subject: [PATCH] fix(console): gate AI surface on the access-filtered agent catalog, not discovery (per-user AI seat) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reverts objectui#1992. `useAiSurfaceEnabled` keys off `GET /api/v1/ai/agents` again (>= 1 agent → AI shows). That route is now access-filtered server-side (framework ADR-0049 / ADR-0068): it returns only the agents the CALLER may chat, so a user WITHOUT the per-user AI seat (`ai_seat`) gets an empty catalog and the ENTIRE AI surface — FAB, `/ai` routes, top-bar + designer "Ask AI" — hides for them, instead of showing a control that 403s on click. #1992 had moved the gate to the deployment-wide discovery `services.ai` capability, which is identical for every user and so cannot express per-user seating — the wrong signal for the AI-seat gate. Community-Edition gating is unaffected (no service-ai → no agents → empty catalog → hidden). Updated the hook docs with a "do NOT flip back to discovery" note so the per-user dimension isn't dropped again. app-shell builds clean; useAiSurface tests (4) green. The catalog's per-user filtering is proven server-side (framework branch feat/ai-seat-catalog-filter): seated user → ['build','ask'], seat-less → []. Co-Authored-By: Claude Opus 4.8 --- .changeset/ai-gate-on-service-ai.md | 23 ----- .changeset/ai-seat-surface-catalog.md | 7 ++ .../src/hooks/__tests__/useAiSurface.test.ts | 53 +++++++---- packages/app-shell/src/hooks/useAiSurface.ts | 87 +++++++++++-------- 4 files changed, 96 insertions(+), 74 deletions(-) delete mode 100644 .changeset/ai-gate-on-service-ai.md create mode 100644 .changeset/ai-seat-surface-catalog.md diff --git a/.changeset/ai-gate-on-service-ai.md b/.changeset/ai-gate-on-service-ai.md deleted file mode 100644 index 9bcab5d63..000000000 --- a/.changeset/ai-gate-on-service-ai.md +++ /dev/null @@ -1,23 +0,0 @@ ---- -"@object-ui/app-shell": patch ---- - -fix(console): gate the AI surface on the `service-ai` capability (discovery), not the agent catalog - -`useAiSurfaceEnabled` now keys off discovery's `services.ai` (`isAiEnabled`) — -i.e. whether the enterprise `@objectstack/service-ai` capability is present — -instead of a non-empty agent catalog. - -`service-ai` is an enterprise capability: a Community-Edition runtime doesn't -ship it, so the framework doesn't register the AI service and discovery reports -`services.ai` unavailable → the whole AI surface hides. An install that has -`service-ai` reports it available → AI shows. The presence of the CAPABILITY -gates, not whether a specific agent happens to be configured yet. - -The earlier catalog-based gating was a workaround for the headless service -reporting itself available in CE; the framework now only registers the AI -service when the host app declares `@objectstack/service-ai` -(objectstack-ai/framework#2311), so discovery is an honest edition signal and -the catalog detour is no longer needed. Everything else stays: the centralized -hook, the `RequireAiSurface` `/ai` route guard, the gated top-bar link + designer -"Ask AI" buttons, and AiChatPage's graceful empty state. diff --git a/.changeset/ai-seat-surface-catalog.md b/.changeset/ai-seat-surface-catalog.md new file mode 100644 index 000000000..ba13cb0a2 --- /dev/null +++ b/.changeset/ai-seat-surface-catalog.md @@ -0,0 +1,7 @@ +--- +'@object-ui/app-shell': patch +--- + +fix(console): gate the AI surface on the access-filtered agent catalog (per-user), not the deployment-wide service-ai capability + +`useAiSurfaceEnabled` keys off `GET /api/v1/ai/agents` again (>= 1 agent → AI shows), reverting objectui#1992. The agent-catalog route is now access-filtered server-side (ADR-0049 / ADR-0068): it returns only the agents the caller may chat, so a user WITHOUT the per-user AI seat (`ai_seat`) gets an empty catalog and the whole AI surface (FAB, `/ai` routes, top-bar + designer "Ask AI") hides for them — instead of showing a control that 403s on click. The discovery `services.ai` flag is deployment-wide and cannot express per-user seating, so it is the wrong signal for the AI-seat gate. Community-Edition gating is unaffected: no service-ai → no agents → empty catalog → hidden. diff --git a/packages/app-shell/src/hooks/__tests__/useAiSurface.test.ts b/packages/app-shell/src/hooks/__tests__/useAiSurface.test.ts index e0d017e98..3a461a5da 100644 --- a/packages/app-shell/src/hooks/__tests__/useAiSurface.test.ts +++ b/packages/app-shell/src/hooks/__tests__/useAiSurface.test.ts @@ -1,34 +1,55 @@ import { describe, it, expect, vi, afterEach } from 'vitest'; import { renderHook } from '@testing-library/react'; -import { useDiscovery } from '@object-ui/react'; +import { useAgents } from '@object-ui/plugin-chatbot'; import { useAiSurfaceEnabled } from '../useAiSurface'; -// The AI surface gates on whether the `service-ai` capability is present, as -// reported by discovery. Mock useDiscovery rather than standing up a data source. -// (The VITE_AI_BASE_URL opt-in branch isn't unit-tested here: vitest fixes -// import.meta.env at startup, so it can't be stubbed per-test; it's a thin -// synchronous env read exercised in the real app.) -vi.mock('@object-ui/react', () => ({ useDiscovery: vi.fn() })); -const mockDiscovery = vi.mocked(useDiscovery); +// The surface is gated on the live agent catalog, so we mock useAgents and +// assert the catalog → enabled/loading mapping (incl. the not-yet-fetched latch). +vi.mock('@object-ui/plugin-chatbot', () => ({ useAgents: vi.fn() })); +const mockAgents = vi.mocked(useAgents); -afterEach(() => mockDiscovery.mockReset()); +function agentsResult(names: string[], isLoading: boolean) { + return { + agents: names.map((name) => ({ name, label: name })), + isLoading, + error: undefined, + refetch: vi.fn(), + }; +} + +afterEach(() => mockAgents.mockReset()); describe('useAiSurfaceEnabled', () => { - it('is enabled when discovery reports service-ai available (enterprise install)', () => { - mockDiscovery.mockReturnValue({ isAiEnabled: true, isLoading: false } as any); + it('is enabled when the catalog serves at least one agent (cloud / agents present)', () => { + mockAgents.mockReturnValue(agentsResult(['ask'], false)); const { result } = renderHook(() => useAiSurfaceEnabled()); expect(result.current).toEqual({ enabled: true, isLoading: false }); }); - it('is disabled — not loading — when service-ai is unavailable (Community Edition: no service-ai)', () => { - mockDiscovery.mockReturnValue({ isAiEnabled: false, isLoading: false } as any); + it('reports loading on the first frame before the catalog fetch has started', () => { + // useAgents starts isLoading=false with an empty list — that is "not fetched + // yet", not "no agents", so the guard must keep waiting (not redirect). + mockAgents.mockReturnValue(agentsResult([], false)); const { result } = renderHook(() => useAiSurfaceEnabled()); - expect(result.current).toEqual({ enabled: false, isLoading: false }); + expect(result.current).toEqual({ enabled: false, isLoading: true }); }); - it('reports loading while discovery is still resolving, so guards do not flash a redirect', () => { - mockDiscovery.mockReturnValue({ isAiEnabled: false, isLoading: true } as any); + it('reports loading while the catalog fetch is in flight', () => { + mockAgents.mockReturnValue(agentsResult([], true)); const { result } = renderHook(() => useAiSurfaceEnabled()); expect(result.current).toEqual({ enabled: false, isLoading: true }); }); + + it('is disabled — not loading — once a fetch resolves empty (Community Edition: no agents)', () => { + // Simulate the real lifecycle: fetch in flight → resolves with an empty + // catalog. The latch records that a fetch ran, so the empty result is now + // definitive and the guard redirects instead of spinning forever. + mockAgents.mockReturnValue(agentsResult([], true)); + const { result, rerender } = renderHook(() => useAiSurfaceEnabled()); + expect(result.current.isLoading).toBe(true); + + mockAgents.mockReturnValue(agentsResult([], false)); + rerender(); + expect(result.current).toEqual({ enabled: false, isLoading: false }); + }); }); diff --git a/packages/app-shell/src/hooks/useAiSurface.ts b/packages/app-shell/src/hooks/useAiSurface.ts index e24e019fc..e87ec5250 100644 --- a/packages/app-shell/src/hooks/useAiSurface.ts +++ b/packages/app-shell/src/hooks/useAiSurface.ts @@ -1,45 +1,52 @@ /** * useAiSurfaceEnabled * - * Single source of truth for "should the in-UI AI surface be shown on this - * deployment?". The console ships under MIT and is edition-agnostic: it decides - * purely at runtime from what the server reports — no `VITE_EDITION` flag, no - * tree-shake. + * Single source of truth for "should the in-UI AI surface be shown — for THIS + * user, on this deployment?". The console ships under MIT and is edition- and + * seat-agnostic at build time; it decides purely at runtime from what the + * server reports — no `VITE_EDITION` flag, no tree-shake. * - * The signal is whether the **`@objectstack/service-ai` capability is present**, - * as reported by discovery (`/discovery` → `services.ai.enabled && - * status === 'available'`, i.e. `isAiEnabled`). + * The signal is **the agent catalog** (`GET /api/v1/ai/agents`): the surface + * shows iff that returns >= 1 agent. The catalog is the right signal because it + * is the ONLY one that is BOTH edition- AND user-aware: * - * `service-ai` is an ENTERPRISE capability: a Community-Edition runtime does not - * depend on it, so the framework never registers the AI service and discovery - * reports `services.ai` unavailable → the whole AI surface hides. An install - * that ships `service-ai` reports it available → AI shows. It is the presence of - * the CAPABILITY that gates, NOT whether any specific agent happens to be - * configured yet (an install with `service-ai` but no agents has AI "available"; - * AiChatPage degrades gracefully if the catalog is empty). + * • The route is access-filtered server-side (ADR-0049 / ADR-0068): it returns + * only the agents the CALLER may chat. A user WITHOUT the per-user AI seat + * (the `ai_seat` permission) gets an EMPTY catalog -> the whole AI surface + * hides for them, instead of showing a button that 403s on click. The + * deployment-wide discovery `services.ai` flag CANNOT express this — it is + * identical for every user — which is exactly why we do NOT gate on it. + * • It is ALSO the honest edition signal: a Community-Edition runtime that + * ships no `@objectstack/service-ai` registers no AI service and persists no + * agents -> empty catalog -> hidden. (The old "headless service reports + * available in CE" worry is moot: empty catalog hides the surface either way.) * - * The framework only registers the AI service when the host app declares - * `@objectstack/service-ai`, so discovery's `services.ai` is an honest edition - * signal (see objectstack-ai/framework#2311). Earlier this hook gated on the - * agent catalog as a workaround for the headless service reporting itself - * available in CE; with #2311 that no longer happens, so discovery is correct. + * ⚠️ Do NOT "simplify" this back to `discovery.services.ai` (isAiEnabled): that + * reintroduces the per-user gap — seat-less users would see the FAB / links and + * hit 403 on click. The per-user AI-seat gate (ADR-0068) DEPENDS on this catalog + * signal. (This reverts objectui#1992, which dropped the per-user dimension.) * - * `VITE_AI_BASE_URL` is an explicit opt-in: it points the console at an external - * AI server and is trusted even when local discovery reports AI unavailable. + * The `VITE_AI_BASE_URL` opt-in flows through naturally: {@link resolveAiApiBase} + * points the catalog fetch at the configured server, so an external AI server + * with reachable agents lights the surface up and an agent-less one keeps it hidden. * - * `isLoading` is surfaced so the `/ai` route guard can wait for discovery to + * `isLoading` is surfaced so the `/ai` route guard can wait for the catalog to * resolve before redirecting — otherwise a stale bookmark would flash a redirect - * to home before the server's answer is in. + * to home before the fetch even starts. Entry-point buttons (FAB, top-bar link, + * designer "Ask AI") ignore it: staying hidden during the brief load is the + * correct, flash-free behaviour for a control that must not appear unless AI can + * actually answer. * * @module */ -import { useDiscovery } from '@object-ui/react'; +import { useRef } from 'react'; +import { useAgents } from '@object-ui/plugin-chatbot'; /** * Resolve the AI service base URL, mirroring AiChatPage / the Home CTAs: * an explicit `VITE_AI_BASE_URL` wins, otherwise `${VITE_SERVER_URL}/api/v1/ai`. - * Shared so every AI fetch (Home catalog, AiChatPage) hits the same URL. + * Shared so every catalog fetch (route guard, layouts, Home) hits the same URL. */ export function resolveAiApiBase(): string { const env = (import.meta as any).env ?? {}; @@ -50,22 +57,32 @@ export function resolveAiApiBase(): string { } export interface AiSurfaceState { - /** True when the AI capability (`service-ai`) is available, so the AI UI shows. */ + /** True when the AI UI should render — the CALLER can reach >= 1 agent (access-filtered). */ enabled: boolean; - /** True until discovery resolves (and no `VITE_AI_BASE_URL` opt-in); guards wait on this. */ + /** True until the agent catalog has resolved; route guards wait on this. */ isLoading: boolean; } /** * Whether the console's AI surface (FAB, `/ai` routes, "Ask AI" affordances) - * should be shown — driven off the presence of the `service-ai` capability in - * discovery. + * should be shown FOR THE CURRENT USER, driven off the access-filtered agent + * catalog (empty for seat-less users -> AI hidden; ADR-0068). */ export function useAiSurfaceEnabled(): AiSurfaceState { - const { isAiEnabled, isLoading } = useDiscovery(); - // An explicit external-AI opt-in is trusted even if local discovery reports AI - // unavailable; it's synchronous, so there's nothing to wait for. - const aiBaseUrlConfigured = Boolean((import.meta as any).env?.VITE_AI_BASE_URL); - if (aiBaseUrlConfigured) return { enabled: true, isLoading: false }; - return { enabled: isAiEnabled, isLoading }; + const { agents, isLoading } = useAgents({ apiBase: resolveAiApiBase() }); + + // useAgents starts `isLoading=false` and only kicks off the fetch in an effect + // a tick later, so the first render's empty list means "not fetched yet", not + // "no agents". Latch whether a fetch has actually been in flight so the route + // guard treats that initial frame as loading (not a definitive empty → redirect). + const fetchStartedRef = useRef(false); + if (isLoading) fetchStartedRef.current = true; + + const enabled = agents.length > 0; + return { + enabled, + // Agents present → resolved/available. Otherwise we're loading until a fetch + // has both started and finished with an empty result. + isLoading: enabled ? false : isLoading || !fetchStartedRef.current, + }; }