refactor(app-shell): one isAiStudioEnabled() accessor for features.aiStudio, replacing two inline spellings

.changeset/ai-studio-accessor-5577.md

@@ -0,0 +1,27 @@
+---
+'@object-ui/app-shell': patch
+---
+
+`features.aiStudio` is now read through one `isAiStudioEnabled()` accessor instead
+of being spelled inline at two call sites (objectui#5577).
+
+`features.marketplace` already had a documented accessor whose docblock is where the
+fail-open doctrine is written down — *"Fails OPEN (`!== false`): a runtime predating
+`/api/v1/runtime/config`, or one whose config fetch failed, keeps the default `true`"*,
+plus the "never infer this from the shape of a failure" warning. `features.aiStudio`
+had no such sibling: `ChatDock` read `getRuntimeConfig().features.aiStudio !== false`
+and `HomePage` read `getRuntimeConfig().features?.aiStudio !== false`, so one doctrine
+had two spellings and neither reader could cite it.
+
+The two spellings were not equivalent. `ChatDock`'s omitted the optional chain, and
+against a runtime-config snapshot whose `features` is absent that read is a TypeError
+rather than a fail-open — the exact shape that crashed 29 tests across four suites in
+PR #5575 before it was corrected. Measured here: no live path can currently deliver
+such a snapshot to `ChatDock` (the module's singleton constructs `features` on every
+write and exports no setter, and no suite mounts the dock's default body under a
+partial stand-in), so this closes a reachable-by-construction crash rather than a live
+one — and it closes it at the source by leaving no inline read to get wrong.
+
+`isAiStudioEnabled()` is an internal module export, matching `isMarketplaceEnabled()`:
+neither is re-exported from `src/index.ts`, so the package's published `exports` surface
+is unchanged.

packages/app-shell/src/console/home/HomePage.tsx

@@ -39,7 +39,7 @@ import { Sparkles, ShieldAlert, X, UploadCloud, MessageSquareText, Hammer, Layou
 import { useMetadataClient } from '../../views/metadata-admin/useMetadata.js';
 import { usePublishAllDrafts } from '../../preview/usePublishAllDrafts.js';
 import { resolveAiApiBase } from '../../hooks/useAiSurface.js';
-import { getRuntimeConfig, isMarketplaceEnabled } from '../../runtime-config.js';
+import { isAiStudioEnabled, isMarketplaceEnabled } from '../../runtime-config.js';
 
 /**
  * Which AI home CTAs to surface, driven by the live agent catalog (the single
@@ -386,19 +386,12 @@ export function HomePage() {
   // hosted-SaaS shape it arrives `false` while the ToolRegistry holds zero
   // authoring handlers and `/api/v1/meta/*` answers 403.
   //
-  // Read inline rather than through a new accessor, so this card adds no
-  // export; lifting this and `ChatDock`'s identical read onto an
-  // `isAiStudioEnabled()` sibling of `isMarketplaceEnabled()` is filed as a
-  // follow-up rather than done here, where it would mean editing four
-  // neighbouring suites' module mocks.
-  //
-  // `features?.` and `!== false` are both load-bearing and are copied from
-  // `isMarketplaceEnabled()`'s body rather than invented: hosts (and four
-  // sibling suites) supply a runtime-config snapshot carrying only `branding`,
-  // so `features` is genuinely absent on real code paths — and an absent flag
-  // must fail OPEN. Withholding the product's front door over an unanswered
-  // question is the worse direction; the server refuses the write regardless.
-  const aiStudioEnabled = getRuntimeConfig().features?.aiStudio !== false;
+  // objectui#5577 — read through the accessor, not inline. `features?.` and
+  // `!== false` are both load-bearing, and the reasoning for each now lives in
+  // exactly one place (`isAiStudioEnabled()`'s docblock) instead of being
+  // retyped per call site: that is what the accessor buys. `ChatDock`'s read
+  // was the same doctrine spelled a second, un-chained way.
+  const aiStudioEnabled = isAiStudioEnabled();
   // Shown wherever an authoring entry point is withheld, so the posture is
   // explained on screen instead of surfacing as a refusal after a filled-in
   // dialog. Localized in all ten packs.

packages/app-shell/src/console/home/__tests__/HomePage.approvalsTarget.test.tsx

@@ -107,6 +107,14 @@ vi.mock('../../../runtime-config', () => ({
   // the gate itself is covered by `HomePage.marketplaceDisabled.test.tsx`,
   // which drives the REAL module instead of this stand-in.
   isMarketplaceEnabled: () => true,
+  // objectui#5577 — same treatment for the AI-authoring gate, which Home now
+  // reads through `isAiStudioEnabled()` rather than inline. An explicit factory
+  // replaces the WHOLE module, so an export it does not list is `undefined` at
+  // the call site — i.e. omitting this line is a TypeError here, not a default.
+  // `true` keeps every case in this file on the pre-existing behaviour; the gate
+  // itself is covered by `HomePage.aiStudioDisabled.test.tsx`, which drives the
+  // REAL module instead of this stand-in.
+  isAiStudioEnabled: () => true,
 }));
 
 import { HomePage } from '../HomePage';

packages/app-shell/src/console/home/__tests__/HomePage.authoringCapabilityGate.test.tsx

@@ -115,6 +115,14 @@ vi.mock('../../../runtime-config', () => ({
   // the gate itself is covered by `HomePage.marketplaceDisabled.test.tsx`,
   // which drives the REAL module instead of this stand-in.
   isMarketplaceEnabled: () => true,
+  // objectui#5577 — same treatment for the AI-authoring gate, which Home now
+  // reads through `isAiStudioEnabled()` rather than inline. An explicit factory
+  // replaces the WHOLE module, so an export it does not list is `undefined` at
+  // the call site — i.e. omitting this line is a TypeError here, not a default.
+  // `true` keeps every case in this file on the pre-existing behaviour; the gate
+  // itself is covered by `HomePage.aiStudioDisabled.test.tsx`, which drives the
+  // REAL module instead of this stand-in.
+  isAiStudioEnabled: () => true,
 }));
 
 import { HomePage } from '../HomePage';

packages/app-shell/src/console/home/__tests__/HomePage.inboxLinksTarget.test.tsx

@@ -109,6 +109,14 @@ vi.mock('../../../runtime-config', () => ({
   // the gate itself is covered by `HomePage.marketplaceDisabled.test.tsx`,
   // which drives the REAL module instead of this stand-in.
   isMarketplaceEnabled: () => true,
+  // objectui#5577 — same treatment for the AI-authoring gate, which Home now
+  // reads through `isAiStudioEnabled()` rather than inline. An explicit factory
+  // replaces the WHOLE module, so an export it does not list is `undefined` at
+  // the call site — i.e. omitting this line is a TypeError here, not a default.
+  // `true` keeps every case in this file on the pre-existing behaviour; the gate
+  // itself is covered by `HomePage.aiStudioDisabled.test.tsx`, which drives the
+  // REAL module instead of this stand-in.
+  isAiStudioEnabled: () => true,
 }));
 
 import { HomePage } from '../HomePage';

packages/app-shell/src/console/home/__tests__/HomePage.notificationDeepLink.test.tsx

@@ -105,6 +105,14 @@ vi.mock('../../../runtime-config', () => ({
   // the gate itself is covered by `HomePage.marketplaceDisabled.test.tsx`,
   // which drives the REAL module instead of this stand-in.
   isMarketplaceEnabled: () => true,
+  // objectui#5577 — same treatment for the AI-authoring gate, which Home now
+  // reads through `isAiStudioEnabled()` rather than inline. An explicit factory
+  // replaces the WHOLE module, so an export it does not list is `undefined` at
+  // the call site — i.e. omitting this line is a TypeError here, not a default.
+  // `true` keeps every case in this file on the pre-existing behaviour; the gate
+  // itself is covered by `HomePage.aiStudioDisabled.test.tsx`, which drives the
+  // REAL module instead of this stand-in.
+  isAiStudioEnabled: () => true,
 }));
 
 import { HomePage } from '../HomePage';

packages/app-shell/src/layout/ChatDock.tsx

@@ -34,7 +34,7 @@ import { AiUsageIndicator } from './AiUsageIndicator.js';
 import { useChatConversation } from '../hooks/index.js';
 import { chatConversationScope, chatProductOfAgent } from '../hooks/chatScope.js';
 import { resolveSurfaceAgent } from '../hooks/surfaceAgent.js';
-import { getRuntimeConfig } from '../runtime-config.js';
+import { isAiStudioEnabled } from '../runtime-config.js';
 import {
   clampDockWidth,
   maximizedDockWidth,
@@ -260,7 +260,7 @@ function ChatDockConversation({
       resolveSurfaceAgent('default', {
         agents,
         appDefaultAgent: defaultAgent,
-        aiStudioEnabled: getRuntimeConfig().features.aiStudio !== false,
+        aiStudioEnabled: isAiStudioEnabled(),
       }),
     [agents, defaultAgent],
   );

packages/app-shell/src/layout/__tests__/ChatDock.partialRuntimeConfig.test.tsx

@@ -0,0 +1,136 @@
+/**
+ * ObjectUI
+ * Copyright (c) 2024-present ObjectStack Inc.
+ *
+ * objectui#5577 — the dock's DEFAULT body under a PARTIAL runtime-config
+ * snapshot.
+ *
+ * `ChatDockConversation` (the body `ChatDockPanel` / `ChatDockMobileSheet` mount
+ * when no `children` override is supplied — i.e. what `ConsoleLayout` renders)
+ * feeds the AI-authoring flag into the ONE surface-agent resolver. It used to
+ * read that flag inline and UN-CHAINED — `getRuntimeConfig().features.aiStudio`
+ * — while `HomePage` read the same flag one file away as `features?.aiStudio`.
+ * One doctrine, two spellings, and only the chained one survives a snapshot
+ * whose `features` is absent: PR #5575 measured the un-chained shape crashing 29
+ * tests across 4 suites before it was corrected.
+ *
+ * These cases pin the corrected shape at the dock. The stand-in is deliberately
+ * NARROW: `importOriginal()` keeps the REAL `isAiStudioEnabled()`, and only
+ * `getRuntimeConfig` is replaced — with the exact partial snapshot four sibling
+ * Home suites already install (`() => ({ branding })`). So a regression to any
+ * inline `getRuntimeConfig().features…` read here turns these red, while the
+ * accessor — which reads the module's own always-complete singleton — stays
+ * green. The assertion is on the VALUE the resolver receives, not merely on
+ * "did not throw": a body that silently stopped mounting would satisfy the
+ * weaker claim while measuring nothing.
+ */
+import '@testing-library/jest-dom/vitest';
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { render, screen } from '@testing-library/react';
+import { ChatDockPanel, ChatDockMobileSheet, type ChatDockState } from '../ChatDock';
+import { resolveSurfaceAgent } from '../../hooks/surfaceAgent';
+
+// PARTIAL — `@object-ui/components`' dialog primitive (which the mobile sheet
+// pulls in) calls `createSafeTranslation` at module scope, so a total stand-in
+// here fails the whole file at import time rather than at any assertion.
+vi.mock('@object-ui/i18n', async (importOriginal) => ({
+  ...(await importOriginal<typeof import('@object-ui/i18n')>()),
+  useObjectTranslation: () => ({
+    t: (key: string, options?: Record<string, unknown>) => String(options?.defaultValue ?? key),
+  }),
+}));
+
+// The partial snapshot under test — `features` genuinely absent, exactly as the
+// four Home suites' stand-in supplies it. Everything else stays REAL: the spy
+// below DELEGATES to the shipped `isAiStudioEnabled()`, so the value these cases
+// assert is the accessor's own answer and only the call is observed.
+const aiStudioSpy = vi.hoisted(() => vi.fn<() => boolean>());
+vi.mock('../../runtime-config', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('../../runtime-config')>();
+  aiStudioSpy.mockImplementation(actual.isAiStudioEnabled);
+  return {
+    ...actual,
+    getRuntimeConfig: () => ({ branding: { productName: 'ObjectStack' } }),
+    isAiStudioEnabled: aiStudioSpy,
+  };
+});
+
+// Spy on the resolver so the flag's VALUE at the seam is observable. The dock
+// computes it in a render-phase `useMemo`, which runs before the empty-catalog
+// early return — so an empty catalog still exercises the read.
+vi.mock('../../hooks/surfaceAgent', () => ({
+  resolveSurfaceAgent: vi.fn(() => undefined),
+}));
+
+// The rest of the chat graph is irrelevant to the flag under test.
+vi.mock('../../console/ai/AiChatPage', () => ({
+  ChatPane: () => null,
+  resolveApiBase: (explicit?: string) => explicit ?? '/api/v1/ai',
+}));
+vi.mock('@object-ui/plugin-chatbot', () => ({
+  useAgents: () => ({ agents: [], isLoading: false, error: undefined }),
+}));
+vi.mock('../../hooks', () => ({
+  useChatConversation: () => ({ conversationId: undefined, initialMessages: [] }),
+}));
+vi.mock('../AiUsageIndicator', () => ({ AiUsageIndicator: () => null }));
+
+const resolverMock = vi.mocked(resolveSurfaceAgent);
+
+function dockState(overrides: Partial<ChatDockState> = {}): ChatDockState {
+  return {
+    expanded: true,
+    width: 420,
+    dragging: false,
+    maximized: false,
+    toggle: vi.fn(),
+    expand: vi.fn(),
+    collapse: vi.fn(),
+    maximize: vi.fn(),
+    restore: vi.fn(),
+    onResizePointerDown: vi.fn(),
+    ...overrides,
+  };
+}
+
+beforeEach(() => {
+  resolverMock.mockClear();
+  aiStudioSpy.mockClear(); // keeps the delegating implementation
+});
+
+describe('ChatDock default body on a partial runtime-config snapshot (objectui#5577)', () => {
+  it('mounts the desktop rail and fails OPEN when `features` is absent', () => {
+    render(<ChatDockPanel dock={dockState()} />);
+
+    expect(screen.getByTestId('chat-dock-panel')).toBeInTheDocument();
+    expect(resolverMock).toHaveBeenCalled();
+    expect(resolverMock.mock.calls[0][1]).toMatchObject({ aiStudioEnabled: true });
+  });
+
+  it('mounts the mobile sheet and fails OPEN when `features` is absent', () => {
+    render(<ChatDockMobileSheet open onOpenChange={vi.fn()} />);
+
+    expect(resolverMock).toHaveBeenCalled();
+    expect(resolverMock.mock.calls[0][1]).toMatchObject({ aiStudioEnabled: true });
+  });
+
+  it('still passes the flag through the `default` surface, not a bare call', () => {
+    // The dock is the console's ambient assistant; the flag only means anything
+    // paired with the surface the resolver is bounded on.
+    render(<ChatDockPanel dock={dockState()} />);
+
+    expect(resolverMock.mock.calls[0][0]).toBe('default');
+  });
+
+  it('asks the ACCESSOR rather than re-spelling the read inline', () => {
+    // Scoped deliberately tighter than the two cases above. Those pin the
+    // crash-closure and stay green for ANY optional-chained inline read
+    // (measured: reverting this call site to `features?.aiStudio !== false`
+    // leaves them passing) — so on their own they say nothing about where the
+    // doctrine lives. This one fails for every inline spelling, chained or not,
+    // which is the actual subject of objectui#5577: ONE doctrine, ONE spelling.
+    render(<ChatDockPanel dock={dockState()} />);
+
+    expect(aiStudioSpy).toHaveBeenCalled();
+  });
+});

packages/app-shell/src/runtime-config.test.ts

@@ -9,7 +9,7 @@
  */
 
 import { describe, it, expect, afterEach, vi } from 'vitest';
-import { initRuntimeConfig, getRuntimeConfig, getPlatformStage, resetRuntimeConfigForTesting } from './runtime-config.js';
+import { initRuntimeConfig, getRuntimeConfig, getPlatformStage, isAiStudioEnabled, isMarketplaceEnabled, resetRuntimeConfigForTesting } from './runtime-config.js';
 
 function mockConfig(features: Record<string, unknown>) {
     vi.stubGlobal('fetch', vi.fn(async () => ({
@@ -68,6 +68,91 @@ function mockBranding(branding: Record<string, unknown>) {
     })) as any);
 }
 
+/**
+ * `isAiStudioEnabled()` — the AI-authoring gate's ONE spelling (objectui#5577).
+ *
+ * `features.aiStudio` used to be read inline at two call sites with two
+ * different spellings (`ChatDock` un-chained, `HomePage` optional-chained), and
+ * the fail-open doctrine was written down only on the `marketplace` sibling. The
+ * accessor is where the doctrine now lives, so these cases pin the doctrine
+ * itself rather than either call site's transcription of it:
+ *
+ *  - fails OPEN on every unanswered question (before init, key absent, fetch
+ *    failed) — withholding a working capability on no answer is the worse
+ *    direction, and the server refuses the write regardless;
+ *  - only the literal `false` closes it;
+ *  - the snapshot the accessor reads always carries `features`, which is why an
+ *    absent flag is a DEFAULT here and never a TypeError.
+ */
+describe('runtime-config isAiStudioEnabled (objectui#5577)', () => {
+    it('fails OPEN before init — a runtime that never answered keeps the capability', () => {
+        resetRuntimeConfigForTesting();
+        expect(isAiStudioEnabled()).toBe(true);
+    });
+
+    it('honours an explicit false from the server', async () => {
+        mockConfig({ aiStudio: false });
+        await initRuntimeConfig();
+        expect(isAiStudioEnabled()).toBe(false);
+    });
+
+    it('stays enabled when the server explicitly says true', async () => {
+        mockConfig({ aiStudio: true });
+        await initRuntimeConfig();
+        expect(isAiStudioEnabled()).toBe(true);
+    });
+
+    it('fails OPEN when the runtime omits the aiStudio key entirely', async () => {
+        mockConfig({ marketplace: true }); // no aiStudio key at all
+        await initRuntimeConfig();
+        expect(isAiStudioEnabled()).toBe(true);
+    });
+
+    it('fails OPEN when the config fetch itself fails', async () => {
+        vi.stubGlobal('fetch', vi.fn(async () => {
+            throw new Error('network down');
+        }) as any);
+        await initRuntimeConfig();
+        expect(isAiStudioEnabled()).toBe(true);
+    });
+
+    it('closes only on the literal false, never on a falsy look-alike', async () => {
+        // `!== false` is the doctrine, not `=== true`: a runtime that sends a
+        // string or a 0 has not said "disabled", so the capability stays.
+        mockConfig({ aiStudio: 'false' });
+        await initRuntimeConfig();
+        expect(isAiStudioEnabled()).toBe(true);
+    });
+
+    it('agrees with its marketplace sibling on the fail-open direction', () => {
+        // One doctrine, one spelling. If these two ever diverge, the accessor
+        // that changed has left the doctrine its docblock claims to carry.
+        resetRuntimeConfigForTesting();
+        expect(isAiStudioEnabled()).toBe(isMarketplaceEnabled());
+        expect(isAiStudioEnabled()).toBe(true);
+    });
+
+    /**
+     * The reachability leg of objectui#5577: the accessor reads the module's own
+     * singleton, and EVERY writer of that singleton constructs `features` as an
+     * object (the initial `{...defaults}`, `applyUpdate`'s spread, and the test
+     * reset). There is no exported setter, so no caller can install a partial
+     * snapshot through the module's API — which is what makes an absent flag a
+     * default rather than the TypeError the un-chained inline read would have
+     * produced. Driven through the real fetch path, not asserted from the source.
+     */
+    it('never hands out a snapshot missing `features`, whatever the server sends', async () => {
+        vi.stubGlobal('fetch', vi.fn(async () => ({
+            ok: true,
+            json: async () => ({ branding: { productName: 'Acme' } }), // no `features` key
+        })) as any);
+        await initRuntimeConfig();
+        expect(getRuntimeConfig().features).toBeDefined();
+        expect(getRuntimeConfig().branding.productName).toBe('Acme');
+        expect(isAiStudioEnabled()).toBe(true);
+    });
+});
+
 describe('runtime-config platform stage', () => {
     it('defaults to preview before init (badge shows out of the box)', () => {
         resetRuntimeConfigForTesting();