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
18 changes: 18 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,24 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
The clear list is now covered by a test that derives the expected set from `docker-compose.yml`
itself, so a forward added without its clear entry fails in CI rather than shipping inert.

- **Restarting from Dashboard > Infrastructure no longer reloads the page into an error.** The restart
modal polls the server and reloads once it answers, but it polled `GET /api/infra/health` — a plain
ping with no knowledge of shutdown. That endpoint keeps answering `200` for the entire drain window
and the engine teardown that follows, so the very first poll, three seconds in, was answered by the
process that had just been asked to exit. The modal declared success and reloaded two seconds later,
by which time the old process was gone and the new one had not yet bound its port — leaving the
operator on a 502/503 from their reverse proxy until they refreshed by hand several times.

The poll now targets `GET /api/health/ready`, which reports `503` the moment draining starts and
keeps reporting it until both databases answer. The page therefore reloads only once the new process
is genuinely serving, and it waits for the databases too rather than only for the port to open.

With the poll now waiting for real readiness, its deadline started to matter for the first time.
It was a fixed 60 attempts at one second, while `POST /api/infra/restart` estimates up to 63 seconds
for a restart that brings up PostgreSQL, Redis and MinIO together — so a full-profile restart could
have reported failure while the stack was still coming up correctly. The deadline is now derived
from the estimate the server already returns, with the old 60 as a floor. (#1019)

## [0.12.2] - 2026-08-01

### Changed
Expand Down
11 changes: 8 additions & 3 deletions dashboard/src/pages/Infrastructure.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
} from 'lucide-react';
import { infraApi, API_BASE_URL } from '../services/api';
import { copyToClipboard } from '../utils/clipboard';
import { restartPollAttempts } from '../utils/restartPoll';
import { useDocumentTitle } from '../hooks/useDocumentTitle';
import { useInfraStatusQuery, useInfraConfigQuery, useEnginesQuery, useCurrentEngineQuery } from '../hooks/queries';
import { PageHeader } from '../components/PageHeader';
Expand Down Expand Up @@ -451,8 +452,12 @@ export function Infrastructure() {

const profilesToRemove = previousProfiles.filter(p => !pendingProfiles.includes(p));

// Kept outside the try: the poll deadline is derived from it, and the restart call is expected to
// fail sometimes (the server may go down before it answers).
let estimatedTime: number | undefined;
try {
const response = await infraApi.restart(pendingProfiles, profilesToRemove);
estimatedTime = response.estimatedTime;
if (response.estimatedTime) setRestartCountdown(response.estimatedTime);
} catch {
// Expected — server shutting down
Expand All @@ -477,12 +482,12 @@ export function Infrastructure() {
});
}, 1000);

checkServerHealth(stopCountdown);
checkServerHealth(stopCountdown, estimatedTime);
};

const checkServerHealth = async (stopCountdown?: () => void) => {
const checkServerHealth = async (stopCountdown?: () => void, estimatedTime?: number) => {
let attempts = 0;
const maxAttempts = 60;
const maxAttempts = restartPollAttempts(estimatedTime);

const check = async () => {
try {
Expand Down
6 changes: 5 additions & 1 deletion dashboard/src/services/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1010,7 +1010,11 @@ export const infraApi = {
method: 'POST',
body: JSON.stringify({ profiles: profiles || [], profilesToRemove: profilesToRemove || [] }),
}),
healthCheck: () => request<{ status: string; timestamp: string }>('/infra/health'),
// Readiness, not the plain /infra/health ping. The restart poll must not be able to latch onto the
// process it just asked to shut down: /infra/health answers 200 for the whole drain and teardown,
// while /health/ready reports 503 as soon as draining starts and stays 503 until both databases
// answer. Public (no API key), like /infra/health.
healthCheck: () => request<{ status: 'ok' | 'error'; details: Record<string, { status: string }> }>('/health/ready'),
// Data migration: export all Data-DB tables (call while still on the OLD database, before switching),
// then import after the switch + restart. Used by the DB-switch migration guard so data isn't lost.
exportData: () =>
Expand Down
56 changes: 56 additions & 0 deletions dashboard/src/utils/restartPoll.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import { test } from 'node:test';
import assert from 'node:assert/strict';
import { readFileSync } from 'node:fs';
import { restartPollAttempts } from './restartPoll.ts';

// The poll runs once a second, so an attempt count IS a deadline in seconds. It used to be a fixed
// 60, which never mattered: the poll hit the still-draining old process and "succeeded" within three
// seconds. Once it waits for real readiness, that 60 becomes the thing that decides whether the
// operator sees the dashboard or an error — and POST /infra/restart estimates up to 63s for a restart
// that brings up postgres + redis + minio. The ceiling must not sit below the server's own estimate.

test('no estimate: keeps the historical 60-attempt deadline', () => {
assert.equal(restartPollAttempts(undefined), 60);
});

test('an estimate the floor already covers does not shorten the deadline', () => {
assert.equal(restartPollAttempts(15), 60);
});

test('a full-profile restart gets more room than the server estimated', () => {
// 15 base + 20 postgres + 13 redis + 15 minio — the worst case infra.controller computes.
assert.equal(restartPollAttempts(63), 126);
});

test('a fractional estimate rounds up rather than truncating below it', () => {
assert.equal(restartPollAttempts(40.5), 81);
});

test('a bogus estimate cannot hang the modal or shorten the deadline', () => {
for (const bogus of [0, -1, NaN, Infinity]) {
assert.equal(restartPollAttempts(bogus), 60, `estimate ${bogus}`);
}
assert.equal(restartPollAttempts(99999), 300);
});

// The two assertions below guard the WIRING, which is where the bug actually lived. A correct helper
// that nothing calls, or a correct deadline pointed at an endpoint that lies during shutdown, both
// pass every assertion above while reproducing #1019 in full. The failure is invisible until an
// operator restarts a production stack, so it has to be caught here.
const read = (path: string): string => readFileSync(new URL(path, import.meta.url), 'utf8');

test('the restart poll targets readiness, not a ping that survives the drain', () => {
const api = read('../services/api.ts');
const healthCheck = api.split('\n').find(line => line.trimStart().startsWith('healthCheck: () =>'));
assert.ok(healthCheck, 'infraApi.healthCheck not found — update this guard, do not delete it');
assert.match(
healthCheck,
/'\/health\/ready'/,
'the poll must hit /health/ready: /infra/health answers 200 throughout the drain, so the poll ' +
'latches onto the dying process and reloads the page into a gap (#1019)',
);
});

test('the poll deadline comes from the helper rather than a bare constant', () => {
assert.match(read('../pages/Infrastructure.tsx'), /const maxAttempts = restartPollAttempts\(/);
});
25 changes: 25 additions & 0 deletions dashboard/src/utils/restartPoll.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/**
* How many one-second readiness polls to allow before a restart is declared failed.
*
* The poll used to latch onto the OLD process — `/infra/health` answers 200 throughout the drain and
* the engine teardown that follows — so it "succeeded" in about three seconds and the fixed 60-attempt
* deadline was never approached. Polling readiness instead means the poll genuinely waits for the new
* process, and the deadline becomes the thing that decides whether the operator sees the dashboard or
* an error screen.
*
* `POST /infra/restart` returns its own `estimatedTime`, which reaches 63s when a restart brings up
* postgres, redis and minio together (and more with services to remove). A deadline below the server's
* own estimate would report failure while the stack is still coming up correctly, so the estimate sets
* the floor and is doubled for margin: the estimate is optimistic, and a container recreate plus
* migrations can overrun it.
*
* The value comes from a server response, so it is clamped at both ends — a missing or absurd estimate
* must not shorten the deadline below the historical 60, nor leave the modal waiting indefinitely.
*/
const MIN_ATTEMPTS = 60;
const MAX_ATTEMPTS = 300;

export function restartPollAttempts(estimatedSeconds?: number): number {
if (!Number.isFinite(estimatedSeconds) || (estimatedSeconds as number) <= 0) return MIN_ATTEMPTS;
return Math.min(MAX_ATTEMPTS, Math.max(MIN_ATTEMPTS, Math.ceil((estimatedSeconds as number) * 2)));
}
Loading