Refactor instance status handling on the frontend

This commit is contained in:
2025-08-27 20:11:21 +02:00
parent b41ebdc604
commit a8f3a8e0f5
12 changed files with 79 additions and 59 deletions

View File

@@ -30,9 +30,9 @@ function App() {
const handleSaveInstance = (name: string, options: CreateInstanceOptions) => { const handleSaveInstance = (name: string, options: CreateInstanceOptions) => {
if (editingInstance) { if (editingInstance) {
updateInstance(editingInstance.name, options); void updateInstance(editingInstance.name, options);
} else { } else {
createInstance(name, options); void createInstance(name, options);
} }
}; };

View File

@@ -46,8 +46,8 @@ function renderApp() {
describe('App Component - Critical Business Logic Only', () => { describe('App Component - Critical Business Logic Only', () => {
const mockInstances: Instance[] = [ const mockInstances: Instance[] = [
{ name: 'test-instance-1', running: false, options: { model: 'model1.gguf' } }, { name: 'test-instance-1', status: 'stopped', options: { model: 'model1.gguf' } },
{ name: 'test-instance-2', running: true, options: { model: 'model2.gguf' } } { name: 'test-instance-2', status: 'running', options: { model: 'model2.gguf' } }
] ]
beforeEach(() => { beforeEach(() => {
@@ -81,7 +81,7 @@ describe('App Component - Critical Business Logic Only', () => {
const user = userEvent.setup() const user = userEvent.setup()
const newInstance: Instance = { const newInstance: Instance = {
name: 'new-test-instance', name: 'new-test-instance',
running: false, status: 'stopped',
options: { model: 'new-model.gguf' } options: { model: 'new-model.gguf' }
} }
vi.mocked(instancesApi.create).mockResolvedValue(newInstance) vi.mocked(instancesApi.create).mockResolvedValue(newInstance)
@@ -118,7 +118,7 @@ describe('App Component - Critical Business Logic Only', () => {
const user = userEvent.setup() const user = userEvent.setup()
const updatedInstance: Instance = { const updatedInstance: Instance = {
name: 'test-instance-1', name: 'test-instance-1',
running: false, status: 'stopped',
options: { model: 'updated-model.gguf' } options: { model: 'updated-model.gguf' }
} }
vi.mocked(instancesApi.update).mockResolvedValue(updatedInstance) vi.mocked(instancesApi.update).mockResolvedValue(updatedInstance)

View File

@@ -27,6 +27,8 @@ const HealthBadge: React.FC<HealthBadgeProps> = ({ health }) => {
return <XCircle className="h-3 w-3" />; return <XCircle className="h-3 w-3" />;
case "unknown": case "unknown":
return <Loader2 className="h-3 w-3 animate-spin" />; return <Loader2 className="h-3 w-3 animate-spin" />;
case "failed":
return <XCircle className="h-3 w-3" />;
} }
}; };
@@ -40,6 +42,8 @@ const HealthBadge: React.FC<HealthBadgeProps> = ({ health }) => {
return "destructive"; return "destructive";
case "unknown": case "unknown":
return "secondary"; return "secondary";
case "failed":
return "destructive";
} }
}; };
@@ -53,6 +57,8 @@ const HealthBadge: React.FC<HealthBadgeProps> = ({ health }) => {
return "Error"; return "Error";
case "unknown": case "unknown":
return "Unknown"; return "Unknown";
case "failed":
return "Failed";
} }
}; };

View File

@@ -24,7 +24,7 @@ function InstanceCard({
editInstance, editInstance,
}: InstanceCardProps) { }: InstanceCardProps) {
const [isLogsOpen, setIsLogsOpen] = useState(false); const [isLogsOpen, setIsLogsOpen] = useState(false);
const health = useInstanceHealth(instance.name, instance.running); const health = useInstanceHealth(instance.name, instance.status);
const handleStart = () => { const handleStart = () => {
startInstance(instance.name); startInstance(instance.name);
@@ -50,13 +50,15 @@ function InstanceCard({
setIsLogsOpen(true); setIsLogsOpen(true);
}; };
const running = instance.status === "running";
return ( return (
<> <>
<Card> <Card>
<CardHeader className="pb-3"> <CardHeader className="pb-3">
<div className="flex items-center justify-between"> <div className="flex items-center justify-between">
<CardTitle className="text-lg">{instance.name}</CardTitle> <CardTitle className="text-lg">{instance.name}</CardTitle>
{instance.running && <HealthBadge health={health} />} {running && <HealthBadge health={health} />}
</div> </div>
</CardHeader> </CardHeader>
@@ -66,7 +68,7 @@ function InstanceCard({
size="sm" size="sm"
variant="outline" variant="outline"
onClick={handleStart} onClick={handleStart}
disabled={instance.running} disabled={running}
title="Start instance" title="Start instance"
data-testid="start-instance-button" data-testid="start-instance-button"
> >
@@ -77,7 +79,7 @@ function InstanceCard({
size="sm" size="sm"
variant="outline" variant="outline"
onClick={handleStop} onClick={handleStop}
disabled={!instance.running} disabled={!running}
title="Stop instance" title="Stop instance"
data-testid="stop-instance-button" data-testid="stop-instance-button"
> >
@@ -108,7 +110,7 @@ function InstanceCard({
size="sm" size="sm"
variant="destructive" variant="destructive"
onClick={handleDelete} onClick={handleDelete}
disabled={instance.running} disabled={running}
title="Delete instance" title="Delete instance"
data-testid="delete-instance-button" data-testid="delete-instance-button"
> >
@@ -122,7 +124,7 @@ function InstanceCard({
open={isLogsOpen} open={isLogsOpen}
onOpenChange={setIsLogsOpen} onOpenChange={setIsLogsOpen}
instanceName={instance.name} instanceName={instance.name}
isRunning={instance.running} isRunning={running}
/> />
</> </>
); );

View File

@@ -29,7 +29,6 @@ const InstanceDialog: React.FC<InstanceDialogProps> = ({
instance, instance,
}) => { }) => {
const isEditing = !!instance; const isEditing = !!instance;
const isRunning = instance?.running || true; // Assume running if instance exists
const [instanceName, setInstanceName] = useState(""); const [instanceName, setInstanceName] = useState("");
const [formData, setFormData] = useState<CreateInstanceOptions>({}); const [formData, setFormData] = useState<CreateInstanceOptions>({});
@@ -114,6 +113,16 @@ const InstanceDialog: React.FC<InstanceDialogProps> = ({
// Check if auto_restart is enabled // Check if auto_restart is enabled
const isAutoRestartEnabled = formData.auto_restart === true; const isAutoRestartEnabled = formData.auto_restart === true;
// Save button label logic
let saveButtonLabel = "Create Instance";
if (isEditing) {
if (instance?.status === "running") {
saveButtonLabel = "Update & Restart Instance";
} else {
saveButtonLabel = "Update Instance";
}
}
return ( return (
<Dialog open={open} onOpenChange={onOpenChange}> <Dialog open={open} onOpenChange={onOpenChange}>
<DialogContent className="sm:max-w-[600px] max-h-[80vh] overflow-hidden flex flex-col"> <DialogContent className="sm:max-w-[600px] max-h-[80vh] overflow-hidden flex flex-col">
@@ -264,11 +273,7 @@ const InstanceDialog: React.FC<InstanceDialogProps> = ({
disabled={!instanceName.trim() || !!nameError} disabled={!instanceName.trim() || !!nameError}
data-testid="dialog-save-button" data-testid="dialog-save-button"
> >
{isEditing {saveButtonLabel}
? isRunning
? "Update & Restart Instance"
: "Update Instance"
: "Create Instance"}
</Button> </Button>
</DialogFooter> </DialogFooter>
</DialogContent> </DialogContent>

View File

@@ -17,13 +17,13 @@ describe('InstanceCard - Instance Actions and State', () => {
const stoppedInstance: Instance = { const stoppedInstance: Instance = {
name: 'test-instance', name: 'test-instance',
running: false, status: 'stopped',
options: { model: 'test-model.gguf' } options: { model: 'test-model.gguf' }
} }
const runningInstance: Instance = { const runningInstance: Instance = {
name: 'running-instance', name: 'running-instance',
running: true, status: 'running',
options: { model: 'running-model.gguf' } options: { model: 'running-model.gguf' }
} }
@@ -301,7 +301,7 @@ afterEach(() => {
it('handles instance with minimal data', () => { it('handles instance with minimal data', () => {
const minimalInstance: Instance = { const minimalInstance: Instance = {
name: 'minimal', name: 'minimal',
running: false, status: 'stopped',
options: {} options: {}
} }
@@ -323,7 +323,7 @@ afterEach(() => {
it('handles instance with undefined options', () => { it('handles instance with undefined options', () => {
const instanceWithoutOptions: Instance = { const instanceWithoutOptions: Instance = {
name: 'no-options', name: 'no-options',
running: true, status: 'running',
options: undefined options: undefined
} }

View File

@@ -44,9 +44,9 @@ describe('InstanceList - State Management and UI Logic', () => {
const mockEditInstance = vi.fn() const mockEditInstance = vi.fn()
const mockInstances: Instance[] = [ const mockInstances: Instance[] = [
{ name: 'instance-1', running: false, options: { model: 'model1.gguf' } }, { name: 'instance-1', status: 'stopped', options: { model: 'model1.gguf' } },
{ name: 'instance-2', running: true, options: { model: 'model2.gguf' } }, { name: 'instance-2', status: 'running', options: { model: 'model2.gguf' } },
{ name: 'instance-3', running: false, options: { model: 'model3.gguf' } } { name: 'instance-3', status: 'stopped', options: { model: 'model3.gguf' } }
] ]
const DUMMY_API_KEY = 'test-api-key-123' const DUMMY_API_KEY = 'test-api-key-123'

View File

@@ -134,7 +134,7 @@ afterEach(() => {
describe('Edit Mode', () => { describe('Edit Mode', () => {
const mockInstance: Instance = { const mockInstance: Instance = {
name: 'existing-instance', name: 'existing-instance',
running: false, status: 'stopped',
options: { options: {
model: 'test-model.gguf', model: 'test-model.gguf',
gpu_layers: 10, gpu_layers: 10,
@@ -184,7 +184,7 @@ afterEach(() => {
}) })
it('shows correct button text for running vs stopped instances', () => { it('shows correct button text for running vs stopped instances', () => {
const runningInstance: Instance = { ...mockInstance, running: true } const runningInstance: Instance = { ...mockInstance, status: 'running' }
const { rerender } = render( const { rerender } = render(
<InstanceDialog <InstanceDialog

View File

@@ -113,8 +113,8 @@ export const InstancesProvider = ({ children }: InstancesProviderProps) => {
setError(null) setError(null)
await instancesApi.start(name) await instancesApi.start(name)
// Update only this instance's running status // Update only this instance's status
updateInstanceInMap(name, { running: true }) updateInstanceInMap(name, { status: "running" })
} catch (err) { } catch (err) {
setError(err instanceof Error ? err.message : 'Failed to start instance') setError(err instanceof Error ? err.message : 'Failed to start instance')
} }
@@ -125,8 +125,8 @@ export const InstancesProvider = ({ children }: InstancesProviderProps) => {
setError(null) setError(null)
await instancesApi.stop(name) await instancesApi.stop(name)
// Update only this instance's running status // Update only this instance's status
updateInstanceInMap(name, { running: false }) updateInstanceInMap(name, { status: "stopped" })
} catch (err) { } catch (err) {
setError(err instanceof Error ? err.message : 'Failed to stop instance') setError(err instanceof Error ? err.message : 'Failed to stop instance')
} }
@@ -137,8 +137,8 @@ export const InstancesProvider = ({ children }: InstancesProviderProps) => {
setError(null) setError(null)
await instancesApi.restart(name) await instancesApi.restart(name)
// Update only this instance's running status // Update only this instance's status
updateInstanceInMap(name, { running: true }) updateInstanceInMap(name, { status: "running" })
} catch (err) { } catch (err) {
setError(err instanceof Error ? err.message : 'Failed to restart instance') setError(err instanceof Error ? err.message : 'Failed to restart instance')
} }

View File

@@ -41,7 +41,7 @@ function TestComponent() {
<div data-testid="instances-count">{instances.length}</div> <div data-testid="instances-count">{instances.length}</div>
{instances.map((instance) => ( {instances.map((instance) => (
<div key={instance.name} data-testid={`instance-${instance.name}`}> <div key={instance.name} data-testid={`instance-${instance.name}`}>
{instance.name}:{instance.running.toString()} {instance.name}:{instance.status}
</div> </div>
))} ))}
@@ -99,8 +99,8 @@ function renderWithProvider(children: ReactNode) {
describe("InstancesContext", () => { describe("InstancesContext", () => {
const mockInstances: Instance[] = [ const mockInstances: Instance[] = [
{ name: "instance1", running: true, options: { model: "model1.gguf" } }, { name: "instance1", status: "running", options: { model: "model1.gguf" } },
{ name: "instance2", running: false, options: { model: "model2.gguf" } }, { name: "instance2", status: "stopped", options: { model: "model2.gguf" } },
]; ];
beforeEach(() => { beforeEach(() => {
@@ -132,10 +132,10 @@ describe("InstancesContext", () => {
expect(screen.getByTestId("loading")).toHaveTextContent("false"); expect(screen.getByTestId("loading")).toHaveTextContent("false");
expect(screen.getByTestId("instances-count")).toHaveTextContent("2"); expect(screen.getByTestId("instances-count")).toHaveTextContent("2");
expect(screen.getByTestId("instance-instance1")).toHaveTextContent( expect(screen.getByTestId("instance-instance1")).toHaveTextContent(
"instance1:true" "instance1:running"
); );
expect(screen.getByTestId("instance-instance2")).toHaveTextContent( expect(screen.getByTestId("instance-instance2")).toHaveTextContent(
"instance2:false" "instance2:stopped"
); );
}); });
}); });
@@ -158,7 +158,7 @@ describe("InstancesContext", () => {
it("creates instance and adds it to state", async () => { it("creates instance and adds it to state", async () => {
const newInstance: Instance = { const newInstance: Instance = {
name: "new-instance", name: "new-instance",
running: false, status: "stopped",
options: { model: "test.gguf" }, options: { model: "test.gguf" },
}; };
vi.mocked(instancesApi.create).mockResolvedValue(newInstance); vi.mocked(instancesApi.create).mockResolvedValue(newInstance);
@@ -181,7 +181,7 @@ describe("InstancesContext", () => {
await waitFor(() => { await waitFor(() => {
expect(screen.getByTestId("instances-count")).toHaveTextContent("3"); expect(screen.getByTestId("instances-count")).toHaveTextContent("3");
expect(screen.getByTestId("instance-new-instance")).toHaveTextContent( expect(screen.getByTestId("instance-new-instance")).toHaveTextContent(
"new-instance:false" "new-instance:stopped"
); );
}); });
}); });
@@ -214,7 +214,7 @@ describe("InstancesContext", () => {
it("updates instance and maintains it in state", async () => { it("updates instance and maintains it in state", async () => {
const updatedInstance: Instance = { const updatedInstance: Instance = {
name: "instance1", name: "instance1",
running: true, status: "running",
options: { model: "updated.gguf" }, options: { model: "updated.gguf" },
}; };
vi.mocked(instancesApi.update).mockResolvedValue(updatedInstance); vi.mocked(instancesApi.update).mockResolvedValue(updatedInstance);
@@ -251,7 +251,7 @@ describe("InstancesContext", () => {
expect(screen.getByTestId("loading")).toHaveTextContent("false"); expect(screen.getByTestId("loading")).toHaveTextContent("false");
// instance2 starts as not running // instance2 starts as not running
expect(screen.getByTestId("instance-instance2")).toHaveTextContent( expect(screen.getByTestId("instance-instance2")).toHaveTextContent(
"instance2:false" "instance2:stopped"
); );
}); });
@@ -262,7 +262,7 @@ describe("InstancesContext", () => {
expect(instancesApi.start).toHaveBeenCalledWith("instance2"); expect(instancesApi.start).toHaveBeenCalledWith("instance2");
// The running state should be updated to true // The running state should be updated to true
expect(screen.getByTestId("instance-instance2")).toHaveTextContent( expect(screen.getByTestId("instance-instance2")).toHaveTextContent(
"instance2:true" "instance2:running"
); );
}); });
}); });
@@ -276,7 +276,7 @@ describe("InstancesContext", () => {
expect(screen.getByTestId("loading")).toHaveTextContent("false"); expect(screen.getByTestId("loading")).toHaveTextContent("false");
// instance1 starts as running // instance1 starts as running
expect(screen.getByTestId("instance-instance1")).toHaveTextContent( expect(screen.getByTestId("instance-instance1")).toHaveTextContent(
"instance1:true" "instance1:running"
); );
}); });
@@ -287,7 +287,7 @@ describe("InstancesContext", () => {
expect(instancesApi.stop).toHaveBeenCalledWith("instance1"); expect(instancesApi.stop).toHaveBeenCalledWith("instance1");
// The running state should be updated to false // The running state should be updated to false
expect(screen.getByTestId("instance-instance1")).toHaveTextContent( expect(screen.getByTestId("instance-instance1")).toHaveTextContent(
"instance1:false" "instance1:stopped"
); );
}); });
}); });
@@ -383,7 +383,7 @@ describe("InstancesContext", () => {
// Test that operations don't interfere with each other // Test that operations don't interfere with each other
const newInstance: Instance = { const newInstance: Instance = {
name: "new-instance", name: "new-instance",
running: false, status: "stopped",
options: {}, options: {},
}; };
vi.mocked(instancesApi.create).mockResolvedValue(newInstance); vi.mocked(instancesApi.create).mockResolvedValue(newInstance);
@@ -411,7 +411,7 @@ describe("InstancesContext", () => {
expect(screen.getByTestId("instances-count")).toHaveTextContent("3"); // Still 3 expect(screen.getByTestId("instances-count")).toHaveTextContent("3"); // Still 3
// But the running state should change // But the running state should change
expect(screen.getByTestId("instance-instance2")).toHaveTextContent( expect(screen.getByTestId("instance-instance2")).toHaveTextContent(
"instance2:true" "instance2:running"
); );
}); });
}); });

View File

@@ -1,14 +1,19 @@
// ui/src/hooks/useInstanceHealth.ts // ui/src/hooks/useInstanceHealth.ts
import { useState, useEffect } from 'react' import { useState, useEffect } from 'react'
import type { HealthStatus } from '@/types/instance' import type { HealthStatus, InstanceStatus } from '@/types/instance'
import { healthService } from '@/lib/healthService' import { healthService } from '@/lib/healthService'
export function useInstanceHealth(instanceName: string, isRunning: boolean): HealthStatus | undefined { export function useInstanceHealth(instanceName: string, instanceStatus: InstanceStatus): HealthStatus | undefined {
const [health, setHealth] = useState<HealthStatus | undefined>() const [health, setHealth] = useState<HealthStatus | undefined>()
useEffect(() => { useEffect(() => {
if (!isRunning) { if (instanceStatus == "stopped") {
setHealth(undefined) setHealth({ status: "unknown", lastChecked: new Date() })
return
}
if (instanceStatus == "failed") {
setHealth({ status: instanceStatus, lastChecked: new Date() })
return return
} }
@@ -17,9 +22,9 @@ export function useInstanceHealth(instanceName: string, isRunning: boolean): Hea
setHealth(healthStatus) setHealth(healthStatus)
}) })
// Cleanup subscription on unmount or when running changes // Cleanup subscription on unmount or when instanceStatus changes
return unsubscribe return unsubscribe
}, [instanceName, isRunning]) }, [instanceName, instanceStatus])
return health return health
} }

View File

@@ -2,14 +2,16 @@ import type { CreateInstanceOptions } from '@/schemas/instanceOptions'
export { type CreateInstanceOptions } from '@/schemas/instanceOptions' export { type CreateInstanceOptions } from '@/schemas/instanceOptions'
export type InstanceStatus = 'running' | 'stopped' | 'failed'
export interface HealthStatus { export interface HealthStatus {
status: 'ok' | 'loading' | 'error' | 'unknown' status: 'ok' | 'loading' | 'error' | 'unknown' | 'failed'
message?: string message?: string
lastChecked: Date lastChecked: Date
} }
export interface Instance { export interface Instance {
name: string; name: string;
running: boolean; status: InstanceStatus;
options?: CreateInstanceOptions; options?: CreateInstanceOptions;
} }