From 8f6b690d594e7772dcb0fc7c6cdbdb68db0b6a3d Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Sun, 11 Oct 2026 03:55:35 -0400 Subject: [PATCH 01/20] feat(instance): add an Operations console for raw operations-API requests A super-user-only Operations tab on every instance and cluster: a Monaco JSON editor (the worker-free JSON language the record editors use) whose body is validated client-side, sent through the entity's existing getInstanceClient client, and answered inline with the status, duration and pretty-printed response or error. - The typed text is the request body; it is parsed only to validate it, so integers past 2^53 reach Harper unchanged. - It is sent at most once: a new skipGatewayRetry request flag opts it out of the shared 502/503/504 replay, which could re-apply a write (the rest of Studio's exposure is #1879). - Only Harper's own reads skip confirmation: the restart gate's naming rule (isReadOperation, extracted so both share it) and membership in the role editor's operations catalog, since a component can register a mutating get_ operation. Everything else confirms with the exact body. - A console `restart` is tracked with markRestarting until the target answers again (waitUntilAnsweringAfterRestart, extracted from restartInstance); `logout` is refused, as it would end the session the rest of Studio is using. - Failures never reach console.error or the global React Query handlers. Anything that reached Harper marks entity queries stale without refetching; a request abandoned mid-flight is reported as possibly applied. The console remounts per entity, and the download is abandoned once progress passes 5 MB. Also: tryParseJson is split out of tryParseRecordJson for reuse, and the engine-position stripper now handles V8's "after JSON at position" wording, which the record editors showed twice too. Closes #1283 Co-Authored-By: Claude Opus 5.5 --- DESIGN.md | 12 +- src/features/instance/InstanceNavBar.test.tsx | 42 ++ src/features/instance/InstanceNavBar.tsx | 16 +- .../Chat/tools/getUserContext/execute.ts | 7 + .../databases/modals/recordEditorJson.test.ts | 7 + .../databases/modals/recordEditorJson.ts | 19 +- .../instance/operations/index.test.tsx | 386 ++++++++++++++++++ src/features/instance/operations/index.tsx | 308 ++++++++++++++ .../operations/operationKinds.test.ts | 22 + .../instance/operations/operationKinds.ts | 11 + .../operations/parseOperationRequest.test.ts | 43 ++ .../operations/parseOperationRequest.ts | 30 ++ src/features/instance/operations/routes.ts | 11 + .../instance/operations/sendOperation.test.ts | 235 +++++++++++ .../instance/operations/sendOperation.ts | 119 ++++++ src/features/instance/routes.ts | 2 + .../api/instance/status/restartInstance.ts | 7 +- src/integrations/api/isReadOperation.test.ts | 45 ++ src/integrations/api/isReadOperation.ts | 12 + .../api/retryGatewayErrors.test.ts | 11 + src/integrations/api/retryGatewayErrors.ts | 12 +- src/lib/restart/restartGate.ts | 14 +- .../__tests__/orgUndefinedRepro.test.ts | 16 + 23 files changed, 1367 insertions(+), 20 deletions(-) create mode 100644 src/features/instance/InstanceNavBar.test.tsx create mode 100644 src/features/instance/operations/index.test.tsx create mode 100644 src/features/instance/operations/index.tsx create mode 100644 src/features/instance/operations/operationKinds.test.ts create mode 100644 src/features/instance/operations/operationKinds.ts create mode 100644 src/features/instance/operations/parseOperationRequest.test.ts create mode 100644 src/features/instance/operations/parseOperationRequest.ts create mode 100644 src/features/instance/operations/routes.ts create mode 100644 src/features/instance/operations/sendOperation.test.ts create mode 100644 src/features/instance/operations/sendOperation.ts create mode 100644 src/integrations/api/isReadOperation.test.ts create mode 100644 src/integrations/api/isReadOperation.ts diff --git a/DESIGN.md b/DESIGN.md index 57ab2ebbf..5c4709e59 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -28,6 +28,16 @@ The `/apis` route is a **custom, in-house API explorer** ([`src/features/instanc - **Try-it-out is credentialed cross-origin `fetch`** (`credentials: 'include'`), the same model Swagger used — which is why the **CORS warning + one-click enable** flow in [`APIDocs.tsx`](src/features/instance/apis/APIDocs.tsx) matters and must be preserved. Auth offers Cookie (session, default) / Basic / Bearer; Basic/Bearer add an `Authorization` header. - **Persistence**: the server + auth selections persist to `localStorage` under `LocalStorageKeys.ApiExplorerSettings`, **keyed by entity id**, so a credential set for one instance never applies to another. +## Operations console (the instance/cluster "Operations" tab) + +[`src/features/instance/operations/`](src/features/instance/operations/) sends one freeform operations-API body through the entity's own `getInstanceClient` client (direct, basic, or proxied — whatever every other page uses). The rules, each pinned by a test there: + +- **The typed text is the request body.** It is parsed only to validate it and read `operation`; re-serializing the parse would round integers past 2^53 (a primary key) before Harper saw them. Axios sends a string body that parses as JSON as-is, only trimmed. +- **At most one send.** It sets `skipGatewayRetry`, because the shared client otherwise replays a 502/503/504 up to three times and a gateway can answer that after Harper applied the operation. Only a request Studio refused before sending counts as unsent. Anything that reached Harper marks data stale, an error answer included, because an arbitrary operation can change something before it fails; and a request abandoned mid-flight (the page unmounted) is reported as possibly applied. +- **Only Harper's own reads skip the confirmation.** [`isBuiltInRead`](src/features/instance/operations/operationKinds.ts) is the restart gate's naming rule ([`isReadOperation`](src/integrations/api/isReadOperation.ts)) _and_ membership in the role editor's catalog of Harper's operations, because a component can register a mutating `get_…`. Everything else — unknown names, every `sql` — confirms, showing the exact body. It guards against a mistyped paste; Harper's `verifyPerms` is what authorizes each operation. +- **Operations Studio has to know about are not sent blind.** A `restart` is tracked with `markRestarting` exactly like the Restart button's (released at once on a 4xx; a 5xx can be a gateway losing the answer to a restart that ran), so the page's reads are held (an instance) or their connectivity errors muted (a cluster, which restarts one member) until `waitUntilAnsweringAfterRestart` sees the target again. `logout` is refused: it would end the session the rest of Studio is still using. Any other operation with such a side effect needs the same treatment rather than a raw send. +- **Nothing leaves the page.** Failures render inline and never pass through `console.error` or the global React Query handlers (RUM reports the former, and bodies carry passwords and secrets). The editor buffer is not persisted, and the console remounts per entity so a confirmation written for one instance can never be sent to another. After a possible write, entity queries are marked stale with `refetchType: 'none'`, never refetched into whatever the operation just did. The download is abandoned once progress passes 5 MB (XHR reports progress about every 50 ms, so this bounds, not caps, what arrives), and display truncates like the API explorer. + ## Cloud authentication presentation All cloud authentication routes use `AuthLayout`; keep form submission, validation, OAuth targets, and error handling in the existing route components/hooks. `AuthInput` keeps `FormControl` directly around the input so label/error associations and React Hook Form refs reach the DOM. React 19 passes `ref` through props; browser tests verify first-invalid-field focus. Password selectors must use an exact label match because the reveal button also has “password” in its accessible name. @@ -39,7 +49,7 @@ The wide hero artwork spans both columns beneath the form card in three independ [`restartTracker.ts`](src/lib/restart/restartTracker.ts) is the one record of which clusters and instances are restarting, so a planned outage doesn't read as a failure. Read it before adding anything that takes an instance down. - **Two sources.** Studio's own Harper `restart` runs mark and release their targets (`markRestarting`, released in `finally`) — central manager never hears about these. Container ops are central manager's: every `getClusterInfo` fetch mirrors its `RESTARTING`/`STARTING` statuses into the tracker — including a one-node cluster whose only member is restarting under an instance-only op, which leaves the cluster's own status `RUNNING`/`PARTIAL`. A new flow that takes an instance down must use one of the two, or its failures toast like before. -- **Reads are held; everything else is refused.** Every `getInstanceClient` client holds a read (`get_`/`search_`/`describe_`/`list_`/`read_`…) while its entity is `down` and sends it on recovery, but refuses a write at once with `ERR_TARGET_RESTARTING` — a write queued for minutes could land after the user gave up on it, or retried it ([`restartGate.ts`](src/lib/restart/restartGate.ts)). Token minting counts as a write on purpose, so a route guard's Fabric Connect attempt falls through to sign-in instead of stalling. Requests that drive or watch a restart must pass `skipRestartGate: true`, or they wait on themselves. A held read is cancelled if the entity's connection generation changes before release: credentials are read at send time, so it would otherwise go out as whoever connected meanwhile. +- **Reads are held; everything else is refused.** Every `getInstanceClient` client holds a read (`get_`/`search_`/`describe_`/`list_`/`read_`…) while its entity is `down` and sends it on recovery, but refuses a write at once with `ERR_TARGET_RESTARTING` — a write queued for minutes could land after the user gave up on it, or retried it ([`restartGate.ts`](src/lib/restart/restartGate.ts); the read rule is [`isReadOperation`](src/integrations/api/isReadOperation.ts), shared with the operations console). Token minting counts as a write on purpose, so a route guard's Fabric Connect attempt falls through to sign-in instead of stalling. Requests that drive or watch a restart must pass `skipRestartGate: true`, or they wait on themselves. A held read is cancelled if the entity's connection generation changes before release: credentials are read at send time, so it would otherwise go out as whoever connected meanwhile. - **Central manager's operations proxy refuses a transitional target outright** — `400 "Cluster is not active"` / `"Instance is not active"` for any `INACTIVE_STATES` status, which includes a _rolling_ restart's `RESTARTING` cluster — and relays the host manager's connection errors as a `500` carrying the raw error text. That is why a proxied client holds for marks with `proxyRefuses`, and why `isConnectivityFailure` counts those two shapes. The set also covers ERROR, FAILED and the terminal states, which no restart releases: a poll through the proxy must be gated on [`isOperationsProxyRefused`](src/components/ui/utils/badgeStatus.tsx), not `isStoppedOrTransitioning`, or it 400s every tick for as long as the page is open (HarperFast/studio#1569). - **Only query errors are muted, and only connectivity-class ones.** Mutations still toast; a 403 or a Harper 500 during a restart still toasts. - **Detection is per tab.** A page learns of a container op started elsewhere only from a cluster-record fetch, and the in-cluster studio pages don't poll the cluster, so they don't notice one started in another tab. diff --git a/src/features/instance/InstanceNavBar.test.tsx b/src/features/instance/InstanceNavBar.test.tsx new file mode 100644 index 000000000..89cc61d68 --- /dev/null +++ b/src/features/instance/InstanceNavBar.test.tsx @@ -0,0 +1,42 @@ +/** @vitest-environment jsdom */ +import { InstanceNavBar } from '@/features/instance/InstanceNavBar'; +import { cleanup, render, screen } from '@testing-library/react'; +import type { ReactNode } from 'react'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +const { permission } = vi.hoisted(() => ({ permission: { canManage: true } })); + +vi.mock('@/hooks/usePermissions', () => ({ useInstanceManagePermission: () => permission.canManage })); +vi.mock('@/config/useInstanceClient', () => ({ useInstanceClientIdParams: () => ({}) })); +vi.mock('@/components/Breadcrumbs', () => ({ Breadcrumbs: () => null })); +vi.mock('@tanstack/react-query', async (importOriginal) => ({ + ...(await importOriginal()), + useQuery: () => ({ data: undefined }), +})); +vi.mock('@tanstack/react-router', () => ({ + Link: ({ to, children, 'aria-label': ariaLabel }: { to: string; children: ReactNode; 'aria-label'?: string }) => ( + {children} + ), + useLoaderData: () => ({ version: '5.3.0' }), + useParams: () => ({ organizationId: 'org-1', clusterId: 'clu-1', instanceId: 'ins-1' }), +})); + +afterEach(cleanup); + +describe('InstanceNavBar', () => { + it('offers the operations console to a super user', () => { + permission.canManage = true; + render(); + + expect(screen.getByRole('link', { name: 'Operations' }).getAttribute('href')) + .toBe('/org-1/clu-1/instance/ins-1/operations'); + }); + + it('hides it from anyone else', () => { + permission.canManage = false; + render(); + + expect(screen.queryByRole('link', { name: 'Operations' })).toBeNull(); + expect(screen.getByRole('link', { name: 'Databases' })).toBeTruthy(); + }); +}); diff --git a/src/features/instance/InstanceNavBar.tsx b/src/features/instance/InstanceNavBar.tsx index 2bf185988..8581cb185 100644 --- a/src/features/instance/InstanceNavBar.tsx +++ b/src/features/instance/InstanceNavBar.tsx @@ -18,7 +18,16 @@ import { wasAReleasedBeforeB } from '@/lib/string/wasAReleasedBeforeB'; import { buildAbsoluteLinkToPage } from '@/lib/urls/buildAbsoluteLinkToPage'; import { useQuery } from '@tanstack/react-query'; import { Link, useLoaderData, useParams } from '@tanstack/react-router'; -import { DatabaseIcon, GaugeIcon, Menu, NotepadTextIcon, PackageIcon, ServerIcon, SettingsIcon } from 'lucide-react'; +import { + DatabaseIcon, + GaugeIcon, + Menu, + NotepadTextIcon, + PackageIcon, + ServerIcon, + SettingsIcon, + SquareTerminalIcon, +} from 'lucide-react'; import { ReactNode, useMemo } from 'react'; interface Link { @@ -67,6 +76,11 @@ export function InstanceNavBar() { name: 'APIs', icon: , }, + canManage && { + to: buildAbsoluteLinkToPage(params, 'operations'), + name: 'Operations', + icon: , + }, canManage && statusAvailable && { to: buildAbsoluteLinkToPage(params, 'status'), icon: , diff --git a/src/features/instance/applications/components/Chat/tools/getUserContext/execute.ts b/src/features/instance/applications/components/Chat/tools/getUserContext/execute.ts index 27b766e17..6ec859174 100644 --- a/src/features/instance/applications/components/Chat/tools/getUserContext/execute.ts +++ b/src/features/instance/applications/components/Chat/tools/getUserContext/execute.ts @@ -29,6 +29,13 @@ export async function execute( }; } + if (url.endsWith('/operations')) { + return { + success: true, + description: 'sending raw requests from the operations API console', + }; + } + if (url.endsWith('/status')) { return { success: true, diff --git a/src/features/instance/databases/modals/recordEditorJson.test.ts b/src/features/instance/databases/modals/recordEditorJson.test.ts index ff73cc132..900cb01c7 100644 --- a/src/features/instance/databases/modals/recordEditorJson.test.ts +++ b/src/features/instance/databases/modals/recordEditorJson.test.ts @@ -34,6 +34,13 @@ describe('tryParseRecordJson', () => { expect(parsed.ok === false && parsed.error.message).not.toMatch(/position|line \d+ column/i); }); + it('drops it from trailing-content errors too, which V8 words "after JSON at position"', () => { + const parsed = tryParseRecordJson('{"a": 1}}'); + + expect(parsed.ok === false && parsed.error.message).toBe('Unexpected non-whitespace character after JSON'); + expect(parsed.ok === false && parsed.error.location).toEqual({ lineNumber: 1, column: 9 }); + }); + // These go to the ops API as `records`, so a primitive parses cleanly and then fails on the // wire in the server's wording. Say it here, where the user can act on it. it('rejects JSON that is well-formed but could never be a record', () => { diff --git a/src/features/instance/databases/modals/recordEditorJson.ts b/src/features/instance/databases/modals/recordEditorJson.ts index a44659450..657cc91bb 100644 --- a/src/features/instance/databases/modals/recordEditorJson.ts +++ b/src/features/instance/databases/modals/recordEditorJson.ts @@ -32,15 +32,24 @@ export function tryParseRecordJson(content: string): ParsedRecordJson { // record rather than an editor the user emptied. return { ok: false, error: { message: 'The editor is empty, so there is nothing to save.' } }; } - let value: unknown; + const parsed = tryParseJson(content); + if (!parsed.ok) { + return parsed; + } + const shapeError = recordShapeError(parsed.value); + return shapeError + ? { ok: false, error: { message: shapeError } } + : { ok: true, value: parsed.value as RecordJsonValue }; +} + +/** Parse `content` as any JSON value; on failure, the parser's reason and (when the engine says) where. */ +export function tryParseJson(content: string): { ok: true; value: unknown } | { ok: false; error: RecordJsonError } { try { - value = JSON.parse(content); + return { ok: true, value: JSON.parse(content) }; } catch (error) { const message = error instanceof Error ? error.message : String(error); return { ok: false, error: { message: withoutEngineLocation(message), location: locate(message, content) } }; } - const shapeError = recordShapeError(value); - return shapeError ? { ok: false, error: { message: shapeError } } : { ok: true, value: value as RecordJsonValue }; } /** One-line rendering of a {@link RecordJsonError}, for a toast description. */ @@ -99,7 +108,7 @@ function isRecord(value: unknown): value is Record { function withoutEngineLocation(message: string): string { return message .replace(/^JSON\.parse:\s*/, '') - .replace(/\s*in JSON at position \d+.*$/i, '') + .replace(/\s*(?:in JSON )?at position \d+.*$/i, '') .replace(/\s*at line \d+ column \d+ of the JSON data\.?$/i, '') .trim(); } diff --git a/src/features/instance/operations/index.test.tsx b/src/features/instance/operations/index.test.tsx new file mode 100644 index 000000000..57345b4f1 --- /dev/null +++ b/src/features/instance/operations/index.test.tsx @@ -0,0 +1,386 @@ +/** @vitest-environment jsdom */ +import { OperationsIndex } from '@/features/instance/operations/index'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'; +import axios, { AxiosError, CanceledError, type InternalAxiosRequestConfig } from 'axios'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const { state, invalidateEntityQueries, markRestarting, releaseRestart, restartSettles } = vi.hoisted(() => { + const releaseRestart = vi.fn(); + const restartSettles = { resolve: () => {}, promise: Promise.resolve() } as { + resolve: () => void; + promise: Promise; + }; + return { + state: { + canManage: true, + user: { username: 'HDB_ADMIN' } as { username: string } | null, + entityId: 'ins-1', + }, + invalidateEntityQueries: vi.fn(), + releaseRestart, + markRestarting: vi.fn(() => releaseRestart), + restartSettles, + }; +}); + +vi.mock('@/lib/monaco/MonacoEditor', () => ({ + Editor: ({ value, onChange }: { value?: string; onChange?: (value: string | undefined) => void }) => ( +