From 7db6454689983d068c7f697b18dcb3bf66216b9d Mon Sep 17 00:00:00 2001 From: os-sales Date: Fri, 21 Aug 2026 22:07:29 +0000 Subject: [PATCH 1/2] refactor(app-shell): give `features.aiStudio` one accessor instead of two inline spellings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `features.marketplace` had a documented accessor carrying the fail-open doctrine; `features.aiStudio` was read inline at two call sites in two different spellings — `ChatDock` un-chained, `HomePage` optional-chained — so neither reader could cite the doctrine and the un-chained one is a TypeError, not a fail-open, against a snapshot whose `features` is absent. That is the shape PR #5575 measured crashing 29 tests. - `isAiStudioEnabled()` sibling of `isMarketplaceEnabled()`, same docblock treatment. - Both call sites moved onto it; no inline `features.aiStudio` read remains. - The four Home suites' module mocks taught the new export (an explicit factory replaces the whole module, so an unlisted export is `undefined` at the call site). - New coverage: the accessor's fail-open doctrine, and the dock's default body under the partial snapshot the un-chained read could not survive. Part of #5577 --- .changeset/ai-studio-accessor-5577.md | 27 ++++ .../app-shell/src/console/home/HomePage.tsx | 21 ++-- .../HomePage.approvalsTarget.test.tsx | 8 ++ .../HomePage.authoringCapabilityGate.test.tsx | 8 ++ .../HomePage.inboxLinksTarget.test.tsx | 8 ++ .../HomePage.notificationDeepLink.test.tsx | 8 ++ packages/app-shell/src/layout/ChatDock.tsx | 4 +- .../ChatDock.partialRuntimeConfig.test.tsx | 116 ++++++++++++++++++ packages/app-shell/src/runtime-config.test.ts | 87 ++++++++++++- packages/app-shell/src/runtime-config.ts | 42 +++++++ 10 files changed, 312 insertions(+), 17 deletions(-) create mode 100644 .changeset/ai-studio-accessor-5577.md create mode 100644 packages/app-shell/src/layout/__tests__/ChatDock.partialRuntimeConfig.test.tsx diff --git a/.changeset/ai-studio-accessor-5577.md b/.changeset/ai-studio-accessor-5577.md new file mode 100644 index 0000000000..dca9479328 --- /dev/null +++ b/.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. diff --git a/packages/app-shell/src/console/home/HomePage.tsx b/packages/app-shell/src/console/home/HomePage.tsx index 2ad50ef7c5..1eab9129f9 100644 --- a/packages/app-shell/src/console/home/HomePage.tsx +++ b/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. diff --git a/packages/app-shell/src/console/home/__tests__/HomePage.approvalsTarget.test.tsx b/packages/app-shell/src/console/home/__tests__/HomePage.approvalsTarget.test.tsx index 5b4f3c4064..67829dd9bb 100644 --- a/packages/app-shell/src/console/home/__tests__/HomePage.approvalsTarget.test.tsx +++ b/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'; diff --git a/packages/app-shell/src/console/home/__tests__/HomePage.authoringCapabilityGate.test.tsx b/packages/app-shell/src/console/home/__tests__/HomePage.authoringCapabilityGate.test.tsx index bd2520324f..806cee8781 100644 --- a/packages/app-shell/src/console/home/__tests__/HomePage.authoringCapabilityGate.test.tsx +++ b/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'; diff --git a/packages/app-shell/src/console/home/__tests__/HomePage.inboxLinksTarget.test.tsx b/packages/app-shell/src/console/home/__tests__/HomePage.inboxLinksTarget.test.tsx index fa0031d212..b7d10bd0b8 100644 --- a/packages/app-shell/src/console/home/__tests__/HomePage.inboxLinksTarget.test.tsx +++ b/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'; diff --git a/packages/app-shell/src/console/home/__tests__/HomePage.notificationDeepLink.test.tsx b/packages/app-shell/src/console/home/__tests__/HomePage.notificationDeepLink.test.tsx index dba76db156..8977d9dd89 100644 --- a/packages/app-shell/src/console/home/__tests__/HomePage.notificationDeepLink.test.tsx +++ b/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'; diff --git a/packages/app-shell/src/layout/ChatDock.tsx b/packages/app-shell/src/layout/ChatDock.tsx index 9b56a70e78..ba756f4bb1 100644 --- a/packages/app-shell/src/layout/ChatDock.tsx +++ b/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], ); diff --git a/packages/app-shell/src/layout/__tests__/ChatDock.partialRuntimeConfig.test.tsx b/packages/app-shell/src/layout/__tests__/ChatDock.partialRuntimeConfig.test.tsx new file mode 100644 index 0000000000..c21dfa2de8 --- /dev/null +++ b/packages/app-shell/src/layout/__tests__/ChatDock.partialRuntimeConfig.test.tsx @@ -0,0 +1,116 @@ +/** + * 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()), + useObjectTranslation: () => ({ + t: (key: string, options?: Record) => 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, so +// `isAiStudioEnabled()` here is the shipped accessor, not a stub of it. +vi.mock('../runtime-config', async (importOriginal) => ({ + ...(await importOriginal()), + getRuntimeConfig: () => ({ branding: { productName: 'ObjectStack' } }), +})); + +// 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 { + 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(); +}); + +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(); + + 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(); + + 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(); + + expect(resolverMock.mock.calls[0][0]).toBe('default'); + }); +}); diff --git a/packages/app-shell/src/runtime-config.test.ts b/packages/app-shell/src/runtime-config.test.ts index 8c5ef64853..9f0df0d199 100644 --- a/packages/app-shell/src/runtime-config.test.ts +++ b/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) { vi.stubGlobal('fetch', vi.fn(async () => ({ @@ -68,6 +68,91 @@ function mockBranding(branding: Record) { })) 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(); diff --git a/packages/app-shell/src/runtime-config.ts b/packages/app-shell/src/runtime-config.ts index a9d50aec4c..a5c9dd3581 100644 --- a/packages/app-shell/src/runtime-config.ts +++ b/packages/app-shell/src/runtime-config.ts @@ -315,6 +315,48 @@ export function isMarketplaceEnabled(): boolean { return current.features?.marketplace !== false; } +/** + * Is AI-driven metadata authoring ("online development") offered by this + * runtime? (objectui#5521 / objectui#5577) + * + * Reads the server's OWN answer — `features.aiStudio`, which the runtime + * derives per request from the same resolution that decides whether the + * metadata-authoring agent is mounted at all. On the composed hosted-SaaS shape + * it arrives `false` while the ToolRegistry holds zero authoring handlers and + * `/api/v1/meta/*` answers 403, so the SPA withholds the authoring entry points + * instead of offering a front door the backend refuses. + * + * Distinct from the PER-PRINCIPAL authoring capability (`useCanAuthorMetadata`): + * this is the DEPLOYMENT's answer ("is authoring offered here at all"), that one + * is the caller's ("may THIS principal author"). Both gates are real and neither + * substitutes for the other. + * + * ⛔ Never infer this from the SHAPE OF A FAILURE. A 403/404 from `/api/v1/meta/*` + * is equally what a permission denial — or a control plane that is merely DOWN — + * produces, and a page that concludes "authoring is disabled on this runtime" + * from it tells an operator their deployment is the problem while the real one is + * their credentials or their upstream. The flag is a property of the runtime's + * own capability set; a broken upstream leaves it `true` and the failure stays a + * failure. + * + * Fails OPEN (`!== false`): a runtime predating `/api/v1/runtime/config`, or one + * whose config fetch failed, keeps the default `true` and the AI authoring + * affordances stay exactly as visible as they were before this gate existed. + * Withholding a working capability on an unanswered question is the worse + * direction — the server refuses the write regardless. + * + * `features?.` is load-bearing, not decoration. It is why this doctrine belongs + * in ONE place instead of being retyped per call site: a caller reached through a + * PARTIAL runtime-config snapshot (a host, or a sibling suite standing the module + * in as `getRuntimeConfig: () => ({ branding })`) sees `features` genuinely + * absent, and reading `.aiStudio` off `undefined` is a TypeError — a crash, not a + * fail-open. PR #5575 measured that exact un-chained shape crashing 29 tests + * across 4 suites before it was corrected. + */ +export function isAiStudioEnabled(): boolean { + return current.features?.aiStudio !== false; +} + /** Test/dev helper. */ export function resetRuntimeConfigForTesting(): void { current = { From e2d01613abf17058774ec3d01e6a319831e77977 Mon Sep 17 00:00:00 2001 From: os-sales Date: Fri, 21 Aug 2026 22:22:06 +0000 Subject: [PATCH 2/2] test(app-shell): make the dock's partial-snapshot probe actually install the snapshot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two corrections found by ablating the probe rather than trusting it: - `vi.mock('../runtime-config')` resolved to `src/layout/runtime-config`, which does not exist. Vitest no-ops an unresolvable factory mock instead of erroring, so the partial snapshot was never installed and the file was a phantom: it passed identically against the un-chained pre-fix read. Corrected to `'../../runtime-config'` — measured red on that read afterwards. - The two crash-closure cases stay green for ANY optional-chained inline read, so they pin the crash and say nothing about where the doctrine lives. Added a case that pins the accessor call itself, scoped to exactly that and measured to fail on a chained inline re-spelling. Part of #5577 --- .../ChatDock.partialRuntimeConfig.test.tsx | 32 +++++++++++++++---- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/packages/app-shell/src/layout/__tests__/ChatDock.partialRuntimeConfig.test.tsx b/packages/app-shell/src/layout/__tests__/ChatDock.partialRuntimeConfig.test.tsx index c21dfa2de8..436a8334e3 100644 --- a/packages/app-shell/src/layout/__tests__/ChatDock.partialRuntimeConfig.test.tsx +++ b/packages/app-shell/src/layout/__tests__/ChatDock.partialRuntimeConfig.test.tsx @@ -41,12 +41,19 @@ vi.mock('@object-ui/i18n', async (importOriginal) => ({ })); // The partial snapshot under test — `features` genuinely absent, exactly as the -// four Home suites' stand-in supplies it. Everything else stays REAL, so -// `isAiStudioEnabled()` here is the shipped accessor, not a stub of it. -vi.mock('../runtime-config', async (importOriginal) => ({ - ...(await importOriginal()), - getRuntimeConfig: () => ({ branding: { productName: 'ObjectStack' } }), -})); +// 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(); + 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 @@ -88,6 +95,7 @@ function dockState(overrides: Partial = {}): ChatDockState { beforeEach(() => { resolverMock.mockClear(); + aiStudioSpy.mockClear(); // keeps the delegating implementation }); describe('ChatDock default body on a partial runtime-config snapshot (objectui#5577)', () => { @@ -113,4 +121,16 @@ describe('ChatDock default body on a partial runtime-config snapshot (objectui#5 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(); + + expect(aiStudioSpy).toHaveBeenCalled(); + }); });