fix(api): replay a gateway error only for reads, so a write is never sent twice

DESIGN.md

@@ -29,7 +29,7 @@ The `/apis` route is a **custom, in-house API explorer** ([`src/features/instanc
 [`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.
+- **At most one send.** It sets `skipGatewayRetry`, because the shared client replays a 502/503/504 for any body whose `operation` reads by name, and a component's `get_…` operation is no proof that it only reads. 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.
@@ -50,6 +50,10 @@ The wide hero artwork spans both columns beneath the form card in three independ
 - **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.
 
+## Instance clients replay only reads
+
+Every `getInstanceClient` client replays a 502/503/504 up to three times (5s/10s/20s), but only for a request [`isReadRequest`](src/integrations/api/isReadOperation.ts) accepts: a GET, or a POST whose body's `operation` is a read by Harper's naming — the restart gate's rule, one predicate for both ([`retryGatewayErrors.ts`](src/integrations/api/retryGatewayErrors.ts)). A write is sent once and its first 5xx is final, because no gateway status proves the write was not applied: Harper itself answers 503 after handing an operation to a worker that exits or overruns. Do not add an exemption for a "usually safe" status or a write that looks idempotent; a caller whose write is safe to repeat retries it itself, where it can say why.
+
 ## Cluster upgrade picker — which versions an existing cluster is offered
 
 [`buildUpgradeVersionOptions`](src/features/clusters/upsert/lib/buildUpgradeVersionOptions.ts) is the one place that decides, and its test file is the spec. Two rules, and they differ on purpose:

src/features/auth/describeAuthFailure.ts

@@ -15,9 +15,9 @@ export const OUTCOME_UNKNOWN_MESSAGE =
 	"We didn't get an answer from the server, so we can't tell whether that went through.";
 
 // 503 alone: the server is declining requests, so it very likely never processed this one and a
-// plain retry is honest. (`curryRetryGatewayErrors` also classes 502/504 as retryable, but it is
-// installed on instance clients only, never on `apiClient` — nothing retries these calls
-// automatically, and the retry this copy invites is the user's own.)
+// plain retry is honest. (`curryRetryGatewayErrors` replays 502/503/504, but only for reads and
+// only on instance clients, never on `apiClient` — nothing retries these calls automatically, and
+// the retry this copy invites is the user's own.)
 const TRANSIENT_STATUSES: ReadonlySet<number> = new Set([503]);
 // Every form using this submits a non-idempotent POST, and each of these means the request had
 // already been handed upstream, so the write may have applied (RFC 9110 §9.2.2 — a retry is only

src/integrations/api/isReadOperation.test.ts

@@ -1,4 +1,4 @@
-import { isReadOperation } from '@/integrations/api/isReadOperation';
+import { isReadOperation, isReadRequest } from '@/integrations/api/isReadOperation';
 import { describe, expect, it } from 'vitest';
 
 describe('isReadOperation', () => {
@@ -43,3 +43,32 @@ describe('isReadOperation', () => {
 		},
 	);
 });
+
+describe('isReadRequest', () => {
+	it.each([
+		['a GET', { method: 'get' }],
+		['an upper-case GET', { method: 'GET' }],
+		['a read body', { method: 'post', data: { operation: 'describe_all' } }],
+		['a read body under an upper-case POST', { method: 'POST', data: { operation: 'describe_all' } }],
+		['a read body axios already serialized', { method: 'post', data: JSON.stringify({ operation: 'get_job' }) }],
+	])('reads %s as a read', (_, config) => {
+		expect(isReadRequest(config)).toBe(true);
+	});
+
+	it.each([
+		['a write body', { method: 'post', data: { operation: 'insert' } }],
+		['a serialized write body', { method: 'post', data: JSON.stringify({ operation: 'drop_table' }) }],
+		['a body that is not JSON', { method: 'post', data: 'describe_all' }],
+		['a JSON null body', { method: 'post', data: 'null' }],
+		['no body', { method: 'post' }],
+		['a DELETE', { method: 'delete' }],
+		['a DELETE with a read body', { method: 'delete', data: { operation: 'describe_all' } }],
+		['a PUT with a serialized read body', { method: 'put', data: JSON.stringify({ operation: 'get_job' }) }],
+		['a PATCH with a read body', { method: 'patch', data: { operation: 'search_by_value' } }],
+		['a read body with no method', { data: { operation: 'describe_all' } }],
+		['a method that is not a string', { method: 42 }],
+		['a read body under a method that is not a string', { method: 42, data: { operation: 'describe_all' } }],
+	])('treats %s as able to change something', (_, config) => {
+		expect(isReadRequest(config)).toBe(false);
+	});
+});

src/integrations/api/isReadOperation.ts

@@ -1,12 +1,36 @@
 /**
  * Reads, by Harper's operation naming: the restart gate holds only these (anything else is refused
- * while the entity restarts), and the operations console asks before sending anything else. Token
- * minting is deliberately not one — held, it would stall a route guard's Fabric Connect attempt behind
- * a notice that cannot mount yet; refused, the guard falls through to sign-in at once.
+ * while the entity restarts), the gateway retry replays only these, and the operations console asks
+ * before sending anything else. Token minting is deliberately not one — held, it would stall a route
+ * guard's Fabric Connect attempt behind a notice that cannot mount yet; refused, the guard falls
+ * through to sign-in at once.
  */
 const READ_OPERATION =
 	/^(?:get_|search_|describe_|list_|read_|user_info$|system_information$|cluster_status$|registration_info$)/;
 
 export function isReadOperation(operation: unknown): boolean {
 	return typeof operation === 'string' && READ_OPERATION.test(operation);
 }
+
+function operationOf(data: unknown): unknown {
+	if (typeof data !== 'string') {
+		return (data as { operation?: unknown } | undefined)?.operation;
+	}
+	// By the time a request has been sent, axios has serialized `config.data` in place; the operations
+	// console also sends raw text.
+	try {
+		return (JSON.parse(data) as { operation?: unknown } | null)?.operation;
+	} catch {
+		return undefined;
+	}
+}
+
+/**
+ * Whether a request is safe to send late (the restart gate) or twice (the gateway retry); anything
+ * unrecognized counts as a write. Harper and central manager's proxy take an operation only from a
+ * POST body, so on any other method an `operation` field is just data.
+ */
+export function isReadRequest({ method, data }: { method?: unknown; data?: unknown }): boolean {
+	const verb = typeof method === 'string' ? method.toLowerCase() : undefined;
+	return verb === 'get' || (verb === 'post' && isReadOperation(operationOf(data)));
+}

src/integrations/api/retryGatewayErrors.integration.test.ts

@@ -0,0 +1,101 @@
+/** @vitest-environment jsdom */
+import { getInstanceClient } from '@/config/getInstanceClient';
+import * as sleepModule from '@/lib/sleep';
+import { AxiosError, type InternalAxiosRequestConfig } from 'axios';
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+
+vi.mock('@/lib/sleep', () => ({
+	sleep: vi.fn().mockResolvedValue(true),
+}));
+
+const OPERATIONS_URL = 'https://ins-123.example.com:9925/';
+
+/**
+ * A target behind a gateway that answers `statuses` in turn, then 200. Axios applies
+ * `validateStatus` inside its own adapters, so this one rejects a non-2xx itself.
+ */
+function gateway(...statuses: number[]) {
+	const sent: Array<{ method?: string; body: unknown }> = [];
+	const client = getInstanceClient({ id: 'ins-123', operationsUrl: OPERATIONS_URL });
+	client.defaults.adapter = async (config: InternalAxiosRequestConfig) => {
+		sent.push({ method: config.method, body: typeof config.data === 'string' ? JSON.parse(config.data) : config.data });
+		const status = statuses.shift() ?? 200;
+		const response = { status, statusText: '', data: { status }, headers: {}, config };
+		if (status >= 400) {
+			throw new AxiosError(
+				`Request failed with status code ${status}`,
+				AxiosError.ERR_BAD_RESPONSE,
+				config,
+				undefined,
+				response,
+			);
+		}
+		return response;
+	};
+	return { client, sent };
+}
+
+function statusOf(error: unknown): number | undefined {
+	return (error as AxiosError).response?.status;
+}
+
+describe('the gateway retry on a real instance client', () => {
+	beforeEach(() => {
+		vi.clearAllMocks();
+	});
+
+	it.each([502, 503, 504])('sends a write once and surfaces its %i', async (status) => {
+		const { client, sent } = gateway(status, status, status, status);
+
+		const error = await client.post('/', { operation: 'insert', records: [{ name: 'a' }] }).catch((e: unknown) => e);
+
+		expect(statusOf(error)).toBe(status);
+		expect(sent).toEqual([{ method: 'post', body: { operation: 'insert', records: [{ name: 'a' }] } }]);
+		expect(sleepModule.sleep).not.toHaveBeenCalled();
+	});
+
+	it.each(['drop_table', 'deploy_component', 'set_configuration', 'restart', 'create_authentication_tokens'])(
+		'never replays %s',
+		async (operation) => {
+			const { client, sent } = gateway(504);
+
+			await expect(client.post('/', { operation })).rejects.toBeInstanceOf(AxiosError);
+			expect(sent).toHaveLength(1);
+		},
+	);
+
+	it.each([502, 503, 504])('replays a read three times on a %i, 5s/10s/20s apart, as before', async (status) => {
+		const { client, sent } = gateway(status, status, status, status);
+
+		const error = await client.post('/', { operation: 'describe_all' }).catch((e: unknown) => e);
+
+		expect(statusOf(error)).toBe(status);
+		expect(sent).toHaveLength(4);
+		expect(sent.every(({ body }) => (body as { operation: string }).operation === 'describe_all')).toBe(true);
+		expect(vi.mocked(sleepModule.sleep).mock.calls).toEqual([[5_000], [10_000], [20_000]]);
+	});
+
+	it('returns the answer to a read that recovers on its replay', async () => {
+		const { client, sent } = gateway(503);
+
+		const { data } = await client.post('/', { operation: 'get_job', id: 'job-1' });
+
+		expect(data).toEqual({ status: 200 });
+		expect(sent).toHaveLength(2);
+	});
+
+	it('replays a GET', async () => {
+		const { client, sent } = gateway(502, 502);
+
+		await expect(client.get('/health')).resolves.toMatchObject({ status: 200 });
+		expect(sent).toHaveLength(3);
+	});
+
+	it('sends a read that opted out once', async () => {
+		const { client, sent } = gateway(503);
+
+		await expect(client.post('/', { operation: 'describe_all' }, { skipGatewayRetry: true })).rejects
+			.toBeInstanceOf(AxiosError);
+		expect(sent).toHaveLength(1);
+	});
+});

src/integrations/api/retryGatewayErrors.test.ts

@@ -7,6 +7,9 @@ vi.mock('@/lib/sleep', () => ({
 	sleep: vi.fn().mockResolvedValue(true),
 }));
 
+/** A read as the interceptor sees it: axios has serialized `config.data` by the time it fails. */
+const READ = { method: 'post', data: JSON.stringify({ operation: 'describe_all' }) };
+
 describe('curryRetryGatewayErrors', () => {
 	let client: Pick<AxiosInstance, 'request'>;
 
@@ -32,9 +35,9 @@ describe('curryRetryGatewayErrors', () => {
 		expect(client.request).not.toHaveBeenCalled();
 	});
 
-	it('rejects and does not retry for non-gateway status codes', async () => {
+	it('rejects and does not retry a read for non-gateway status codes', async () => {
 		const handler = curryRetryGatewayErrors(client);
-		const config: AxiosRequestConfig & { __retryCount?: number } = {};
+		const config: AxiosRequestConfig & { __retryCount?: number } = { ...READ };
 		const err = { response: { status: 500 }, config };
 
 		await expect(handler(err)).rejects.toBe(err);
@@ -44,9 +47,9 @@ describe('curryRetryGatewayErrors', () => {
 		expect(config.__retryCount).toBeUndefined();
 	});
 
-	it('retries on 502 and increments __retryCount with initial delay on first retry', async () => {
+	it('retries a read on 502 and increments __retryCount with initial delay on first retry', async () => {
 		const handler = curryRetryGatewayErrors(client);
-		const config: AxiosRequestConfig & { __retryCount?: number } = {};
+		const config: AxiosRequestConfig & { __retryCount?: number } = { ...READ };
 		const err = { response: { status: 502 }, config };
 
 		const result = await handler(err);
@@ -59,9 +62,9 @@ describe('curryRetryGatewayErrors', () => {
 		expect(result).toEqual({ ok: true });
 	});
 
-	it('retries on 503 with exponential backoff: more backoff on second retry', async () => {
+	it('retries a read on 503 with exponential backoff: more backoff on second retry', async () => {
 		const handler = curryRetryGatewayErrors(client);
-		const config: AxiosRequestConfig & { __retryCount?: number } = { __retryCount: 1 };
+		const config: AxiosRequestConfig & { __retryCount?: number } = { ...READ, __retryCount: 1 };
 		const err = { response: { status: 503 }, config };
 
 		await handler(err);
@@ -72,9 +75,9 @@ describe('curryRetryGatewayErrors', () => {
 		expect(client.request).toHaveBeenCalledTimes(1);
 	});
 
-	it('retries on 504 with the largest backoff on third retry', async () => {
+	it('retries a read on 504 with the largest backoff on third retry', async () => {
 		const handler = curryRetryGatewayErrors(client);
-		const config: AxiosRequestConfig & { __retryCount?: number } = { __retryCount: 2 };
+		const config: AxiosRequestConfig & { __retryCount?: number } = { ...READ, __retryCount: 2 };
 		const err = { response: { status: 504 }, config };
 
 		await handler(err);
@@ -85,9 +88,37 @@ describe('curryRetryGatewayErrors', () => {
 		expect(client.request).toHaveBeenCalledTimes(1);
 	});
 
-	it.each([502, 503, 504])('never replays a %i for a request that opted out', async (status) => {
+	it.each([
+		['an object body', { operation: 'insert', records: [] }],
+		['a serialized body', JSON.stringify({ operation: 'drop_table', table: 'dog' })],
+		['token minting', JSON.stringify({ operation: 'create_authentication_tokens' })],
+		['a body that is not JSON', 'operation=describe_all'],
+		['no body', undefined],
+	])('never replays a write on any gateway status: %s', async (_, data) => {
+		const handler = curryRetryGatewayErrors(client);
+		for (const status of [502, 503, 504]) {
+			const config: AxiosRequestConfig & { __retryCount?: number } = { method: 'post', data };
+			const err = { response: { status }, config };
+
+			await expect(handler(err)).rejects.toBe(err);
+			expect(config.__retryCount).toBeUndefined();
+		}
+		expect(sleepModule.sleep).not.toHaveBeenCalled();
+		expect(client.request).not.toHaveBeenCalled();
+	});
+
+	it('replays a GET, which carries no operation', async () => {
+		const handler = curryRetryGatewayErrors(client);
+		const config: AxiosRequestConfig & { __retryCount?: number } = { method: 'get' };
+
+		await handler({ response: { status: 503 }, config });
+
+		expect(client.request).toHaveBeenCalledWith(config);
+	});
+
+	it.each([502, 503, 504])('never replays a %i for a read that opted out', async (status) => {
 		const handler = curryRetryGatewayErrors(client);
-		const config: AxiosRequestConfig & { __retryCount?: number } = { skipGatewayRetry: true };
+		const config: AxiosRequestConfig & { __retryCount?: number } = { ...READ, skipGatewayRetry: true };
 		const err = { response: { status }, config };
 
 		await expect(handler(err)).rejects.toBe(err);
@@ -98,7 +129,7 @@ describe('curryRetryGatewayErrors', () => {
 
 	it('stops retrying when max retries reached (>= 3)', async () => {
 		const handler = curryRetryGatewayErrors(client);
-		const config: AxiosRequestConfig & { __retryCount?: number } = { __retryCount: 3 };
+		const config: AxiosRequestConfig & { __retryCount?: number } = { ...READ, __retryCount: 3 };
 		const err = { response: { status: 502 }, config };
 
 		await expect(handler(err)).rejects.toBe(err);

src/integrations/api/retryGatewayErrors.ts

@@ -1,16 +1,23 @@
+import { isReadRequest } from '@/integrations/api/isReadOperation';
 import { sleep } from '@/lib/sleep';
 import { AxiosInstance } from 'axios';
 
 declare module 'axios' {
 	interface AxiosRequestConfig {
 		/**
-		 * Send at most once. A 502/504 can arrive after Harper has applied the operation, so a request
-		 * whose idempotence is unknown (the operations console's freeform body) must not be replayed.
+		 * Send at most once, even a read. The operations console sets it: its freeform body can name a
+		 * component's operation, and a `get_` name from a component is no proof that it only reads.
 		 */
 		skipGatewayRetry?: boolean;
 	}
 }
 
+/**
+ * Replays a 502/503/504 up to three times, for reads only. None of the three means the request went
+ * unapplied — a proxy answers for an upstream that may already have it, and Harper itself answers 503
+ * when the worker running an operation exits or overruns — so a write is sent once and its first
+ * answer is final.
+ */
 export function curryRetryGatewayErrors(client: Pick<AxiosInstance, 'request'>) {
 	// eslint-disable-next-line @typescript-eslint/no-explicit-any
 	return async (error: any) => {
@@ -20,7 +27,7 @@ export function curryRetryGatewayErrors(client: Pick<AxiosInstance, 'request'>)
 			return Promise.reject(error);
 		}
 
-		const shouldRetry = status === 502 || status === 503 || status === 504;
+		const shouldRetry = (status === 502 || status === 503 || status === 504) && isReadRequest(config);
 		if (!shouldRetry) {
 			return Promise.reject(error);
 		}