Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions DESIGN.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,15 @@ The same rule decides what a failure says. [`isUnansweredWrite`](src/integration
- **These toasts are never collapsed** onto a shared id: each warns about a different write, and a read timing out alongside one must not replace it through `request-timeout`.
- **A read that is costly to repeat** stays a read here and opts out of the replay instead (`package_component`, `skipGatewayRetry`).

## A restart Harper accepted never reads as failed

Harper's answer to the restart request is its only verdict on the restart: `restart` answers before it tears down (harper `bin/restart.ts` schedules the teardown 50ms out so the reply can go first) and `restart_service` once its job has started (`executeJob`). So [`restartInstance`](src/integrations/api/instance/status/restartInstance.ts) rejects with the request's own error only when that request failed; after a 2xx, a target Studio never sees answer the `user_info` watch successfully rejects with [`RestartUnconfirmedError`](src/integrations/api/instance/status/restartUnconfirmed.ts), which reads "Not back yet", never "Failed", and is never a reason to send the restart again.

- **The watch outlasts the usual recovery time** (one settle, then two probe schedules), and `onSlowToReturn` fires once between them so a caller can say it is still waiting. Every restart mark uses `RESTART_MARK_TTL_MS`, derived from the same constants, so no mark lapses while its watch still runs; the probes skip the gateway replay for the same reason.
- **Only a successful probe counts as back, never a 401/403.** Through central manager's operations proxy a 403 is CM refusing the caller before it contacts the instance (`notAllowed()` in its `HDBInstance.js`), so it says nothing about the target; reading it as "back" would report a restart Studio never saw finish. That is also why the copy says Studio "couldn't confirm it came back", not that the target never answered.
- **A one-at-a-time loop stops at a member that is not back** (cluster restart, rolling config update): restarting the next one could leave no member serving. Both loops also wait on a member whose restart got no verdict, and stop if that follow-up watch gives up too: it may have restarted and still be down, or, in a config update, restarted onto a configuration that broke it. Either report names the members it left alone, and it, like the no-verdict warning, outranks Cancel, which would otherwise hide the warning not to restart again; the report then says which members Cancel left. A Cancel clicked during a member's configuration write stops before its restart, and the report says the configuration may be pending there. On a 5.2+ cluster the config update never fans out from a canary that is not back, since it never proved it can run the configuration; a stop after the fan-out keeps the note that the members it left may already have the new configuration saved.
- **The chat tools return `success: true`** for it: their action was carried out, and `false` invites the agent to repeat it.

## 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:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
import { templates } from '@/features/instance/applications/components/NewApplication/templates';
import { addComponent } from '@/integrations/api/instance/applications/addComponent';
import { restartInstance } from '@/integrations/api/instance/status/restartInstance';
import { RestartUnconfirmedError } from '@/integrations/api/instance/status/restartUnconfirmed';
import { findBy } from '@/lib/arrays/findBy';
import { emitToListeners } from '@/lib/events/listener';
import { toKebabCase } from '@/lib/string/to-kebab-case';
import { inputSchema } from '@harperfast/agent-tools/tools/createApp/inputSchema';
import { toast } from 'sonner';
import { z } from 'zod';
import { ExecuteParams } from '../../types/executeParams';
import { describeToolError } from '../describeToolError';
import { describeToolError, toastToolFailure } from '../describeToolError';
import { Output } from './output';

export async function execute(
Expand Down Expand Up @@ -38,6 +39,7 @@ export async function execute(
template: template.npm || template.githubUrl,
});
} catch (err) {
toastToolFailure(toastId, err);
return {
success: false,
message: `Error: ${describeToolError(err)}`,
Expand All @@ -59,6 +61,17 @@ export async function execute(
replicated: instanceClientParams.entityType === 'cluster',
});
} catch (err) {
toastToolFailure(toastId, err);
if (err instanceof RestartUnconfirmedError) {
// The app exists either way; `success: false` would invite creating it again.
return {
success: true,
message: `App "${name}" created. Harper accepted the restart that loads it, but Studio couldn't confirm `
+ "the instance came back, so it may still be starting. Don't create the app or restart again until "
+ 'its status shows it is back.',
webURL: baseURL,
};
}
return {
success: false,
message: `Error: ${describeToolError(err)}`,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
import { isUnansweredWrite, UNANSWERED_WRITE_MESSAGE } from '@/integrations/api/unansweredWrite';
import { describeError } from '@/react-query/queryClient';
import { toast } from 'sonner';

export function describeToolError(err: unknown): string {
return isUnansweredWrite(err) ? UNANSWERED_WRITE_MESSAGE : String(err);
}

/** A loading toast never dismisses itself, so a failed tool must replace its progress toast. */
export function toastToolFailure(toastId: string | number, err: unknown) {
const { title, description } = describeError(err);
toast.error(title, { id: toastId, description, duration: 10_000 });
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { restartInstance } from '@/integrations/api/instance/status/restartInstance';
import { RestartUnconfirmedError } from '@/integrations/api/instance/status/restartUnconfirmed';
import { inputSchema } from '@harperfast/agent-tools/tools/restartHTTPService/inputSchema';
import { toast } from 'sonner';
import { z } from 'zod';
import { ExecuteParams } from '../../types/executeParams';
import { describeToolError } from '../describeToolError';
import { describeToolError, toastToolFailure } from '../describeToolError';
import { Output } from './output';

export async function execute(
Expand All @@ -21,6 +22,17 @@ export async function execute(
replicated: instanceClientParams.entityType === 'cluster',
});
} catch (err) {
toastToolFailure(toastId, err);
if (err instanceof RestartUnconfirmedError) {
// `success` is whether the tool's own action was carried out, and Harper accepted the restart.
return {
success: true,
message: "Harper accepted the restart, but Studio couldn't confirm the HTTP service came back, so it may "
+ "still be starting. Don't restart it again until its status shows it is back, or it goes down a "
+ 'second time.',
webURL: baseURL,
};
}
return {
success: false,
message: `Error: ${describeToolError(err)}`,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
import {
RESTART_UNCONFIRMED_MESSAGE,
RESTART_UNCONFIRMED_TITLE,
RestartUnconfirmedError,
} from '@/integrations/api/instance/status/restartUnconfirmed';
import { AxiosError } from 'axios';
import { toast } from 'sonner';
import { beforeEach, describe, expect, it, vi } from 'vitest';
import type { ExecuteParams } from '../types/executeParams';
import { execute as createApp } from './createApp/execute';
import { execute as restartHTTPService } from './restartHTTPService/execute';

vi.mock('@/integrations/api/instance/status/restartInstance', () => ({ restartInstance: vi.fn() }));
vi.mock('@/integrations/api/instance/applications/addComponent', () => ({ addComponent: vi.fn() }));
vi.mock('@/lib/events/listener', () => ({ emitToListeners: vi.fn() }));
vi.mock('sonner', () => ({
toast: { loading: vi.fn(() => 'tool-toast'), error: vi.fn(), success: vi.fn() },
}));

import { addComponent } from '@/integrations/api/instance/applications/addComponent';
import { restartInstance } from '@/integrations/api/instance/status/restartInstance';

function params<T>(input: T): ExecuteParams<T> {
return {
input,
instanceClientParams: { entityType: 'instance', entityId: 'ins-1' } as ExecuteParams['instanceClientParams'],
baseURL: 'https://ins-1.example.com',
params: {},
};
}

const notBackYet = () => new RestartUnconfirmedError(new AxiosError('Network Error', AxiosError.ERR_NETWORK));

function refused() {
const config = { method: 'post', data: { operation: 'restart_service' } } as never;
return new AxiosError('Request failed with status code 403', AxiosError.ERR_BAD_REQUEST, config, undefined, {
status: 403,
statusText: '',
data: { error: 'This operation is restricted to super users' },
headers: {},
config,
});
}

beforeEach(() => {
vi.clearAllMocks();
vi.mocked(addComponent).mockResolvedValue({} as never);
});

describe('restartHTTPService', () => {
it('tells the agent a restart Harper accepted is not back yet, and not to restart it again', async () => {
vi.mocked(restartInstance).mockRejectedValue(notBackYet());

const output = await restartHTTPService(params({}));

expect(output.success).toBe(true);
expect(output.message).not.toMatch(/^Error/);
expect(output.message).toContain("Don't restart it again");
expect(restartInstance).toHaveBeenCalledOnce();
expect(toast.error).toHaveBeenCalledWith(
RESTART_UNCONFIRMED_TITLE,
expect.objectContaining({
id: 'tool-toast',
description: RESTART_UNCONFIRMED_MESSAGE,
}),
);
});

it('still reports a refused restart as an error, and clears its progress toast', async () => {
vi.mocked(restartInstance).mockRejectedValue(refused());

const output = await restartHTTPService(params({}));

expect(output.success).toBe(false);
expect(output.message).toMatch(/^Error: /);
expect(toast.error).toHaveBeenCalledWith('Error', expect.objectContaining({ id: 'tool-toast' }));
});
});

describe('createApp', () => {
it('keeps the fact that the app was created when its restart is not back yet', async () => {
vi.mocked(restartInstance).mockRejectedValue(notBackYet());

const output = await createApp(params({ name: 'My App', type: 'vanilla' as const }));

expect(output.success).toBe(true);
expect(output.message).toMatch(/^App "my-app" created\. Harper accepted the restart/);
expect(output.message).toContain("Don't create the app or restart again");
expect(addComponent).toHaveBeenCalledOnce();
});

it('clears its progress toast when the app could not be created', async () => {
vi.mocked(addComponent).mockRejectedValue(new Error('Component my-app already exists'));

const output = await createApp(params({ name: 'My App', type: 'vanilla' as const }));

expect(output.success).toBe(false);
expect(restartInstance).not.toHaveBeenCalled();
expect(toast.error).toHaveBeenCalledWith(expect.any(String), expect.objectContaining({ id: 'tool-toast' }));
});
});
1 change: 1 addition & 0 deletions src/features/instance/operations/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ vi.mock('@/lib/restart/restartTracker', async (importOriginal) => ({
markRestarting,
}));
vi.mock('@/integrations/api/instance/status/restartInstance', () => ({
RESTART_MARK_TTL_MS: 600_000,
waitUntilAnsweringAfterRestart: () => restartSettles.promise,
}));
vi.mock('@tanstack/react-router', () => ({
Expand Down
8 changes: 5 additions & 3 deletions src/features/instance/operations/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@ import { MAX_RESPONSE_BYTES, OperationResult, sendOperation } from '@/features/i
import { useInstanceAuth } from '@/hooks/useAuth';
import { useMonacoTheme } from '@/hooks/useMonacoTheme';
import { useInstanceManagePermission } from '@/hooks/usePermissions';
import { waitUntilAnsweringAfterRestart } from '@/integrations/api/instance/status/restartInstance';
import {
RESTART_MARK_TTL_MS,
waitUntilAnsweringAfterRestart,
} from '@/integrations/api/instance/status/restartInstance';
import { humanFileSize } from '@/lib/humanFileSize';
import { Editor } from '@/lib/monaco/MonacoEditor';
import { WORKER_FREE_JSON_LANGUAGE_ID } from '@/lib/monaco/workerFreeJsonLanguage';
Expand All @@ -38,7 +41,6 @@ import { useEffect, useRef, useState } from 'react';
const OPERATIONS_API_DOCS = 'https://docs.harperdb.io/docs/developers/operations-api/';
const INITIAL_REQUEST = '{\n\t"operation": "describe_all"\n}\n';
const CONFIRM_PREVIEW_CHARS = 4_000;
const RESTART_TTL_MS = 5 * 60_000;
const LOGOUT_REFUSAL = 'Sign out from Studio instead: a logout sent from here would end the session the rest of '
+ 'Studio is still using, without Studio knowing.';

Expand Down Expand Up @@ -100,7 +102,7 @@ function OperationsConsole({ instanceClient, entityId, noun, username }: {
// Tracked like the Restart button's, so the page's polls ride out the outage instead of failing
// into the global error handlers.
const releaseRestart = request.operation === 'restart'
? markRestarting([entityId], { reach: noun === 'instance' ? 'down' : 'rolling', ttlMs: RESTART_TTL_MS })
? markRestarting([entityId], { reach: noun === 'instance' ? 'down' : 'rolling', ttlMs: RESTART_MARK_TTL_MS })
: undefined;
try {
const result = await sendOperation(instanceClient, request.text, {
Expand Down
Loading
Loading