From 2e14c6eda69fc8f8d34091d7774d2d480bfa2afb Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 6 Sep 2026 12:54:10 +0000 Subject: [PATCH] test(app-shell): serve batch 5's three app-shell probes from doubles (objectui#7307) Three of the four remaining network-escape rows now answer from recording doubles instead of a real socket, and their lines leave both KNOWN_ESCAPES and PINNED_LEDGER in lockstep (4 -> 1 on each side). Traced at the guard's attribution point rather than guessed: - FlowNodeInspector.inactiveRetained -> FlowReferenceField.tsx:389 -> MetadataClient.list -> GET /api/v1/meta/object, answered as an empty `{ type, items: [] }` registry; - StudioDesignSurface.designerRegistryMissing -> StudioDesignSurface.tsx:3797 -> GET /api/v1/automation/_status, answered as an empty `{ data: { flows: [] } }` roster; - studioSurfaceContext -> useChatConversation.ts:609 -> POST /api/v1/ai/conversations, answered as one empty conversation, plus the GET resume route the hook's localStorage cache makes cases 2-4 take. Each double is a router, not a sink: it records every URL and its afterEach fails on any URL outside the routes it serves; cleanup() runs before vi.unstubAllGlobals() (objectui#7439 ordering). Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01FhBNJcLRZLe8M87VcUgpKr --- .changeset/network-escapes-batch5.md | 8 ++ ...lowNodeInspector.inactiveRetained.test.tsx | 77 ++++++++++++++++- ...gnSurface.designerRegistryMissing.test.tsx | 71 +++++++++++++++- .../__tests__/studioSurfaceContext.test.tsx | 82 ++++++++++++++++++- .../__tests__/network-escape-ledger.test.ts | 3 - vitest.setup.network-escape-guard.ts | 6 -- 6 files changed, 233 insertions(+), 14 deletions(-) create mode 100644 .changeset/network-escapes-batch5.md diff --git a/.changeset/network-escapes-batch5.md b/.changeset/network-escapes-batch5.md new file mode 100644 index 0000000000..1967c76f96 --- /dev/null +++ b/.changeset/network-escapes-batch5.md @@ -0,0 +1,8 @@ +--- +--- + +Test-only (objectui#7307 batch 5): three app-shell suites now serve their +`GET /api/v1/meta/object`, `GET /api/v1/automation/_status` and +`/api/v1/ai/conversations` probes from recording doubles instead of a real +socket, and their three rows leave the network-escape ledger. No published +runtime code changes, so nothing to release. diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.inactiveRetained.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.inactiveRetained.test.tsx index 187e1957b9..86b5c8f8c3 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.inactiveRetained.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.inactiveRetained.test.tsx @@ -10,7 +10,7 @@ * inert is this notice beside it. */ -import { describe, it, expect, vi, afterEach } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { render, screen, cleanup, fireEvent, within } from '@testing-library/react'; vi.mock('../previews/useFlowNodePalette', () => ({ @@ -24,7 +24,80 @@ vi.mock('../previews/useObjectFields', () => ({ import { FlowNodeInspector } from './FlowNodeInspector'; import type { MetadataSelection } from '../preview-registry'; -afterEach(cleanup); +/* ── The `meta/object` double (objectui#7307) ───────────────────────── + * `FlowNodeInspector` renders `FlowReferenceField` for every reference-kind key + * on the selected node, and that field resolves its combobox options through + * `useMetadataListOptions` — `MetadataClient.list(type)`, i.e. + * `GET /api/v1/meta/object` over the authenticated wrapper, which resolves the + * GLOBAL `fetch` at call time (`packages/auth/src/createAuthenticatedFetch.ts`, + * the bare `await fetch(input, ...)`). Under happy-dom that global is a real HTTP + * client and the document URL defaults to `http://localhost:3000`, so the + * relative path resolved to a live socket. Traced from the guard's attribution + * point: `FlowReferenceField.tsx:389` → `metadata-client.ts:764` → that wrapper. + * + * Answered from a RECORDING double — the shape objectui#5225 settled on, carried + * by `packages/plugin-report/src/__tests__/DatasetReportRenderer.test.tsx` and by + * this burn-down's earlier batches. Deliberately NOT a blanket network stub: it + * records every URL it is handed and `afterEach` fails on any URL outside the + * route it serves, so an escape to somewhere else reds here instead of vanishing + * into the hook's `.catch`. + * + * What it answers, and why that changes no assertion here: an EMPTY registry, in + * the `{ type, items: [] }` envelope the server sends and `MetadataClient.list` + * parses (it also accepts a bare array; both parse to the same rows). Empty is + * load-bearing — the failing request landed in the hook's `.catch`, which sets + * `{ options: [], loading: false }`, so an empty registry yields byte-identical + * output to what these cases have always rendered, while a seeded one would put + * options into every reference combobox in the tree. The route is matched on the + * PATHNAME because `MetadataClient.list` appends `?package=` / `?preview=draft` + * for scoped callers; the full URL is what gets recorded. + * + * `headers` is part of the answer, not decoration: the authenticated wrapper + * reads `response.headers.get('set-auth-token')` on every API call before the + * caller ever sees the body. + * ──────────────────────────────────────────────────────────── */ + +const META_OBJECT_ROUTE = '/api/v1/meta/object'; + +/** Every URL this file's renders handed the global `fetch`, in request order. */ +let metaCalls: string[] = []; + +/** The route key of a recorded URL: its pathname, without the scope query. */ +const routeOf = (url: string) => url.split('?')[0]; + +/** Serve `GET /api/v1/meta/object` as an empty registry; record everything. */ +function installMetaObjectDouble() { + metaCalls = []; + vi.stubGlobal( + 'fetch', + vi.fn(async (input: unknown) => { + const url = String( + input && typeof input === 'object' && 'url' in input ? (input as { url: unknown }).url : input, + ); + metaCalls.push(url); + if (routeOf(url) !== META_OBJECT_ROUTE) { + return { ok: false, status: 404, headers: new Headers(), json: async () => ({}) }; + } + return { ok: true, status: 200, headers: new Headers(), json: async () => ({ type: 'object', items: [] }) }; + }), + ); +} + +beforeEach(installMetaObjectDouble); + +afterEach(() => { + // The double is a router, not a sink: an escape to any OTHER endpoint fails + // here instead of vanishing into `useMetadataListOptions`'s `.catch`. + expect(metaCalls.filter((url) => routeOf(url) !== META_OBJECT_ROUTE)).toEqual([]); + // Unmount BEFORE restoring the real `fetch` — this replaces the bare + // `afterEach(cleanup)` that used to stand here, it does not drop it. Vitest + // runs `afterEach` hooks in reverse registration order, so this file's + // teardown runs before the root setup's RTL cleanup: unstubbing first would + // leave the tree mounted with the real global back in place, and a mount + // effect settling in that window escapes again (objectui#7439). + cleanup(); + vi.unstubAllGlobals(); +}); function draftWith(config: Record, type = 'approval') { return { nodes: [{ id: 'gate', type, label: 'Gate', config }], edges: [] }; diff --git a/packages/app-shell/src/views/studio-design/StudioDesignSurface.designerRegistryMissing.test.tsx b/packages/app-shell/src/views/studio-design/StudioDesignSurface.designerRegistryMissing.test.tsx index 563ae61af3..4625ecf5f6 100644 --- a/packages/app-shell/src/views/studio-design/StudioDesignSurface.designerRegistryMissing.test.tsx +++ b/packages/app-shell/src/views/studio-design/StudioDesignSurface.designerRegistryMissing.test.tsx @@ -44,7 +44,7 @@ import '@testing-library/jest-dom/vitest'; import * as React from 'react'; -import { describe, it, expect, vi, afterEach } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { render, screen, fireEvent, cleanup, waitFor } from '@testing-library/react'; import { MemoryRouter } from 'react-router-dom'; @@ -97,7 +97,74 @@ import { listMetadataInspectorTypes, getMetadataInspector } from '../metadata-ad import { getMetadataDefaultInspector } from '../metadata-admin/default-inspector-registry'; import { getStudioCanvasPreview } from './studio-canvas-preview'; -afterEach(cleanup); +/* ── The `automation/_status` double (objectui#7307) ────────────────── + * `AutomationsPillar` reads the engine's live per-flow runtime state from a + * mount effect — `StudioDesignSurface.tsx:3797`, a bare global `fetch` of + * `GET /api/v1/automation/_status` with no `apiFetch` seam on the path. Under + * happy-dom that global is a real HTTP client and the document URL defaults to + * `http://localhost:3000`, so the relative path resolved to a live socket. The + * effect's read is best-effort by construction (its `catch` comment: "offline / + * older backend → no dots"), which is why the Automations case below stayed + * green while the request always failed. + * + * Answered from a RECORDING double — the shape objectui#5225 settled on, carried + * by `packages/plugin-report/src/__tests__/DatasetReportRenderer.test.tsx` and by + * this burn-down's earlier batches. Deliberately NOT a blanket network stub: it + * records every URL it is handed and `afterEach` fails on any URL outside the + * route it serves, so an escape to somewhere else reds here instead of vanishing + * into that `catch`. + * + * What it answers, and why that changes no assertion here: a known-EMPTY runtime + * roster, in the `{ data: { flows: [...] } }` envelope the effect reads first + * (it also accepts a bare `{ flows }`; both parse to the same rows). Empty is + * load-bearing — the effect turns each row into a status DOT on the flow rail, + * and the failing request left `flowStatus` at `{}` with no dots at all, so an + * empty roster renders exactly what these cases have always rendered, while a + * seeded one would add a dot for `nightly` to the Automations tableau this file + * pins. Routes are matched on the PATHNAME; the full URL is what gets recorded. + * ──────────────────────────────────────────────────────────── */ + +const AUTOMATION_STATUS_ROUTE = '/api/v1/automation/_status'; + +/** Every URL this file's renders handed the global `fetch`, in request order. */ +let statusCalls: string[] = []; + +/** The route key of a recorded URL: its pathname, without any query. */ +const routeOf = (url: string) => url.split('?')[0]; + +/** Serve `GET /api/v1/automation/_status` as an empty roster; record everything. */ +function installStatusDouble() { + statusCalls = []; + vi.stubGlobal( + 'fetch', + vi.fn(async (input: unknown) => { + const url = String( + input && typeof input === 'object' && 'url' in input ? (input as { url: unknown }).url : input, + ); + statusCalls.push(url); + if (routeOf(url) !== AUTOMATION_STATUS_ROUTE) { + return { ok: false, status: 404, headers: new Headers(), json: async () => ({}) }; + } + return { ok: true, status: 200, headers: new Headers(), json: async () => ({ data: { flows: [] } }) }; + }), + ); +} + +beforeEach(installStatusDouble); + +afterEach(() => { + // The double is a router, not a sink: an escape to any OTHER endpoint fails + // here instead of vanishing into the effect's best-effort `catch`. + expect(statusCalls.filter((url) => routeOf(url) !== AUTOMATION_STATUS_ROUTE)).toEqual([]); + // Unmount BEFORE restoring the real `fetch` — this replaces the bare + // `afterEach(cleanup)` that used to stand here, it does not drop it. Vitest + // runs `afterEach` hooks in reverse registration order, so this file's + // teardown runs before the root setup's RTL cleanup: unstubbing first would + // leave the tree mounted with the real global back in place, and a mount + // effect settling in that window escapes again (objectui#7439). + cleanup(); + vi.unstubAllGlobals(); +}); /** * Assert the registries really are empty — **with a control that MUST hit**. diff --git a/packages/app-shell/src/views/studio-design/__tests__/studioSurfaceContext.test.tsx b/packages/app-shell/src/views/studio-design/__tests__/studioSurfaceContext.test.tsx index 43a9dc0bfd..13a7debc29 100644 --- a/packages/app-shell/src/views/studio-design/__tests__/studioSurfaceContext.test.tsx +++ b/packages/app-shell/src/views/studio-design/__tests__/studioSurfaceContext.test.tsx @@ -11,7 +11,7 @@ */ import '@testing-library/jest-dom/vitest'; import * as React from 'react'; -import { afterEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { cleanup, render } from '@testing-library/react'; import { MemoryRouter, Route, Routes } from 'react-router-dom'; @@ -47,8 +47,88 @@ vi.mock('@object-ui/plugin-chatbot', async (importOriginal) => { import { StudioCopilotConversation } from '../StudioAiCopilot'; +/* ── The `ai/conversations` double (objectui#7307) ──────────────────── + * Every `renderAt` below mounts the real `StudioCopilotConversation`, whose + * `useChatConversation` resolve effect mints a thread for the signed-in user on + * mount: `POST /api/v1/ai/conversations` through the GLOBAL `fetch` + * (`hooks/useChatConversation.ts:609`, no `apiFetch` seam on the path). Under + * happy-dom that global is a real HTTP client and the document URL defaults to + * `http://localhost:3000`, so the relative path resolved to a live socket — once + * per case, four in the file. The resolve's `catch` is deliberately conservative + * (it keeps the surface as it was), which is why these cases stayed green while + * the mint always failed. + * + * Answered from a RECORDING double — the shape objectui#5225 settled on, carried + * by `packages/plugin-report/src/__tests__/DatasetReportRenderer.test.tsx` and by + * this burn-down's earlier batches. Deliberately NOT a blanket network stub: it + * records every URL it is handed and `afterEach` fails on any URL outside the + * routes it serves. + * + * TWO routes, because a mint that SUCCEEDS is resumable and the hook resumes it. + * `useChatConversation` caches the minted id in `localStorage` + * (`writeCache` → `readCache`), and happy-dom keeps that store for the whole + * file — so case 1 mints and cases 2-4 resume, reading + * `GET /api/v1/ai/conversations/{THE_MINTED_ID}` instead. That second route was + * MEASURED, not assumed: serving only the mint made this file's own + * router assertion red naming that exact URL. Both answer the same empty + * `ServerConversation` (`{ id, messages: [] }` — the shape `createConversation` + * and `fetchConversation` both cast their body to), so the fake server is + * self-consistent: one thread, minted once, resumed thereafter. + * + * Why an empty thread changes no assertion here: this file asserts ONE thing per + * case — the `surfaceContext` prop the pane receives, derived from the URL alone. + * `ChatPane` is a capture stub, the conversation never reaches an assertion, and + * the resolve settles in a microtask AFTER each synchronous case body has already + * read `capturedProps`. A SEEDED thread would hydrate `initialMessages` into that + * same stub for no assertion's benefit. Routes are matched on the PATHNAME; the + * full URL is what gets recorded. + * ─────────────────────────────────────────────────── */ + +/** The one thread this fake server owns: minted by case 1, resumed by 2-4. */ +const CONVERSATION = { id: 'conv_studio_copilot', messages: [] as unknown[] }; + +/** `POST` here mints; `GET .../{id}` resumes. Nothing else is served. */ +const MINT_ROUTE = '/api/v1/ai/conversations'; +const RESUME_ROUTE = `${MINT_ROUTE}/${CONVERSATION.id}`; +const SERVED_ROUTES = new Set([MINT_ROUTE, RESUME_ROUTE]); + +/** Every URL this file's renders handed the global `fetch`, in request order. */ +let aiCalls: string[] = []; + +/** The route key of a recorded URL: its pathname, without any query. */ +const routeOf = (url: string) => url.split('?')[0]; + +/** Serve the two conversation routes as one empty thread; record everything. */ +function installConversationsDouble() { + aiCalls = []; + vi.stubGlobal( + 'fetch', + vi.fn(async (input: unknown) => { + const url = String( + input && typeof input === 'object' && 'url' in input ? (input as { url: unknown }).url : input, + ); + aiCalls.push(url); + if (!SERVED_ROUTES.has(routeOf(url))) { + return { ok: false, status: 404, headers: new Headers(), json: async () => ({}) }; + } + return { ok: true, status: 200, headers: new Headers(), json: async () => CONVERSATION }; + }), + ); +} + +beforeEach(installConversationsDouble); + afterEach(() => { + // The double is a router, not a sink: an escape to any OTHER endpoint fails + // here instead of vanishing into the resolve effect's `catch`. + expect(aiCalls.filter((url) => !SERVED_ROUTES.has(routeOf(url)))).toEqual([]); + // Unmount BEFORE restoring the real `fetch`. Vitest runs `afterEach` hooks in + // reverse registration order, so this file's teardown runs before the root + // setup's RTL cleanup: unstubbing first would leave the tree mounted with the + // real global back in place, and a mount effect settling in that window + // escapes again (objectui#7439). cleanup(); + vi.unstubAllGlobals(); capturedProps = {}; }); diff --git a/scripts/__tests__/network-escape-ledger.test.ts b/scripts/__tests__/network-escape-ledger.test.ts index ca845c2328..33f34338df 100644 --- a/scripts/__tests__/network-escape-ledger.test.ts +++ b/scripts/__tests__/network-escape-ledger.test.ts @@ -39,10 +39,7 @@ const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../ * and must be done in lockstep with `KNOWN_ESCAPES`. */ const PINNED_LEDGER: readonly string[] = [ - 'packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.inactiveRetained.test.tsx', 'packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.specKeys.test.tsx', - 'packages/app-shell/src/views/studio-design/StudioDesignSurface.designerRegistryMissing.test.tsx', - 'packages/app-shell/src/views/studio-design/__tests__/studioSurfaceContext.test.tsx', ]; describe('network-escape ledger (objectui#6640) is shrink-only', () => { diff --git a/vitest.setup.network-escape-guard.ts b/vitest.setup.network-escape-guard.ts index 4205b9ff87..60640c1acd 100644 --- a/vitest.setup.network-escape-guard.ts +++ b/vitest.setup.network-escape-guard.ts @@ -113,14 +113,8 @@ const ESCAPE_ORIGIN = /^https?:\/\/(?:127\.0\.0\.1|localhost):3000(?:\/|$)/; * ONLY SHRINKS. The comment on each line is the endpoint it reached. */ export const KNOWN_ESCAPES: ReadonlySet = new Set([ - // /api/v1/meta/object - 'packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.inactiveRetained.test.tsx', // /api/v1/meta/object 'packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.specKeys.test.tsx', - // /api/v1/automation/_status - 'packages/app-shell/src/views/studio-design/StudioDesignSurface.designerRegistryMissing.test.tsx', - // /api/v1/ai/conversations - 'packages/app-shell/src/views/studio-design/__tests__/studioSurfaceContext.test.tsx', ]); type Escape = { file: string; test: string; url: string };