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
71 changes: 50 additions & 21 deletions frontend/src/components/config/config-editor.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useBlocker, useNavigate } from "@tanstack/react-router";
import { useBlocker } from "@tanstack/react-router";
import {
AlertTriangle,
CheckCircle2,
Expand Down Expand Up @@ -30,11 +30,12 @@ import {
DialogTitle,
} from "#/components/ui/dialog";
import { WorkspaceLayout } from "#/components/ui/workspace-layout";
import { useAuth } from "#/lib/auth";
import { ApiRequestError } from "#/lib/api";
import {
type ConfigDocument,
type ConfigWarning,
loadConfigDocument,
loadServiceStatus,
scheduleRestart,
} from "#/lib/config-api";

Expand Down Expand Up @@ -118,13 +119,11 @@ function ConfigWorkspace({
backgroundError,
}: ConfigWorkspaceProps) {
const draft = useConfigDraft(initialDocument);
const { setAuth } = useAuth();
const navigate = useNavigate();
const [mobileNavigationOpen, setMobileNavigationOpen] = useState(false);
const [restartOpen, setRestartOpen] = useState(false);
const [restartBusy, setRestartBusy] = useState(false);
const [actionMessage, setActionMessage] = useState("");
const allowNavigation = useRef(false);
const restartMonitorGeneration = useRef(0);
const { t } = useTranslation();
const activeSection = useMemo(
() =>
Expand All @@ -136,7 +135,6 @@ function ConfigWorkspace({

const blocker = useBlocker({
shouldBlockFn: ({ current, next }) =>
!allowNavigation.current &&
draft.isDirty &&
current.pathname === "/config" &&
next.pathname !== "/config",
Expand All @@ -145,29 +143,59 @@ function ConfigWorkspace({
withResolver: true,
});

async function handleConfirmSave() {
const result = await draft.confirmSave();
if (!result) return;
if (result.session_invalidated) {
allowNavigation.current = true;
useEffect(
() => () => {
restartMonitorGeneration.current += 1;
},
[],
);

async function monitorRestart() {
const generation = restartMonitorGeneration.current + 1;
restartMonitorGeneration.current = generation;
for (let attempt = 0; attempt < 60; attempt += 1) {
if (attempt > 0) {
await new Promise((resolve) => window.setTimeout(resolve, 500));
}
if (restartMonitorGeneration.current !== generation) return;
try {
setAuth({ authenticated: false });
await navigate({
to: "/login",
search: {
notice: "config_saved_restart_scheduled",
},
});
} finally {
allowNavigation.current = false;
const status = await loadServiceStatus();
if (restartMonitorGeneration.current !== generation) return;
if (status.restart_status === "command_failed") {
setActionMessage(t("config.status.restartCommandFailed"));
return;
}
if (
status.restart_status === "command_completed" ||
status.restart_status === "idle"
) {
return;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
} catch (statusError) {
// A 401 is handled globally and proves that a new process with changed
// credentials is serving requests. Other failures are inconclusive.
if (
statusError instanceof ApiRequestError &&
statusError.status === 401
) {
return;
}
}
return;
}
if (restartMonitorGeneration.current === generation) {
setActionMessage(t("config.status.restartStatusUnavailable"));
}
}

async function handleConfirmSave() {
const result = await draft.confirmSave();
if (!result) return;
setActionMessage(
result.requires_restart
? t("config.status.savedRestart")
: t("config.status.saved"),
);
if (result.restart_scheduled) void monitorRestart();
}

async function handleRestart() {
Expand All @@ -177,6 +205,7 @@ function ConfigWorkspace({
await scheduleRestart();
setActionMessage(t("config.status.restartScheduled"));
setRestartOpen(false);
void monitorRestart();
} catch (restartError) {
setActionMessage(
t("config.status.restartFailed", {
Expand Down
4 changes: 1 addition & 3 deletions frontend/src/components/config/use-config-draft.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,9 +202,7 @@ export function useConfigDraft(initialDocument: ConfigDocument) {
setBaseline(structuredClone(snapshot));
setDraft(structuredClone(snapshot));
setBaseRevision(result.revision);
setRestartRequired(
result.requires_restart && !result.restart_scheduled,
);
setRestartRequired(result.requires_restart);
draftVersion.current += 1;
setCheck({ status: "idle" });
setPreview({ status: "closed" });
Expand Down
117 changes: 100 additions & 17 deletions frontend/src/config-editor.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,14 @@ type PreviewOverrides = Partial<{
hasChanges: boolean;
passwordChange: boolean;
warnings: string[];
restartStatus: "idle" | "scheduled" | "command_completed" | "command_failed";
restartStatusSequence: Array<
| "idle"
| "scheduled"
| "command_completed"
| "command_failed"
| "network_error"
>;
}>;

function previewResponse(previewOverrides: PreviewOverrides = {}) {
Expand Down Expand Up @@ -140,6 +148,9 @@ function installApi(
nextPreview?: () => Promise<Response>,
) {
const requests: Array<{ url: string; init?: RequestInit }> = [];
const restartStatusSequence = [
...(previewOverrides.restartStatusSequence ?? []),
];
const fetchMock = vi.fn(
async (input: RequestInfo | URL, init?: RequestInit) => {
const url = String(input);
Expand Down Expand Up @@ -169,7 +180,25 @@ function installApi(
revision: "saved-revision",
requires_restart: true,
restart_scheduled: url.includes("restart_after_save=true"),
session_invalidated: url.includes("restart_after_save=true"),
session_invalidated: false,
}),
{ status: 200, headers: { "Content-Type": "application/json" } },
);
}
if (url === "/api/status") {
const nextStatus = restartStatusSequence.shift();
if (nextStatus === "network_error") {
throw new Error("temporary connection failure");
}
return new Response(
JSON.stringify({
version: "test",
uptime_seconds: 1,
api_bind: "0.0.0.0",
api_port: 8080,
database_path: "/tmp/sms-relayed.sqlite",
restart_status:
nextStatus ?? previewOverrides.restartStatus ?? "scheduled",
}),
{ status: 200, headers: { "Content-Type": "application/json" } },
);
Expand Down Expand Up @@ -530,32 +559,25 @@ describe("ConfigEditor workspace", () => {
await screen.findByText("Check failed: invalid device name");
});

test("uses combined save and restart for a password change then signs out", async () => {
test("keeps the current session when a password-change restart command fails", async () => {
const setAuth = vi.fn();
const leavingConfig = {
current: { pathname: "/config" },
next: { pathname: "/login" },
};
const { requests } = installApi({
passwordChange: true,
warnings: ["password_change"],
restartStatus: "command_failed",
});
render(<EditorHarness initialSection="api" setAuth={setAuth} />);

fireEvent.change(await screen.findByLabelText("Password"), {
target: { value: "new-password" },
});
expect(routerMocks.shouldBlockFn(leavingConfig)).toBe(true);
routerMocks.navigate.mockImplementation(async () => {
expect(routerMocks.shouldBlockFn(leavingConfig)).toBe(false);
});
fireEvent.click(screen.getByRole("button", { name: "Save" }));
fireEvent.click(
await screen.findByRole("button", { name: "Save and schedule restart" }),
);

await waitFor(() =>
expect(setAuth).toHaveBeenCalledWith({ authenticated: false }),
await screen.findByText(
"Service restart command failed. Your current session is still active; retry restart.",
);
const saveRequest = requests.find(
(request) => request.init?.method === "PUT",
Expand All @@ -564,12 +586,73 @@ describe("ConfigEditor workspace", () => {
expect(
requests.some((request) => request.url === "/api/service/restart"),
).toBe(false);
expect(routerMocks.navigate).toHaveBeenCalledWith(
expect.objectContaining({
to: "/login",
search: { notice: "config_saved_restart_scheduled" },
}),
expect(requests.some((request) => request.url === "/api/status")).toBe(
true,
);
expect(setAuth).not.toHaveBeenCalled();
expect(routerMocks.navigate).not.toHaveBeenCalled();
expect(screen.getByText(/Restart required/)).toBeTruthy();
});

test("continues monitoring after a transient restart status failure", async () => {
const setAuth = vi.fn();
const { requests } = installApi({
passwordChange: true,
warnings: ["password_change"],
restartStatusSequence: ["network_error", "command_failed"],
});
render(<EditorHarness initialSection="api" setAuth={setAuth} />);

fireEvent.change(await screen.findByLabelText("Password"), {
target: { value: "new-password" },
});
fireEvent.click(screen.getByRole("button", { name: "Save" }));
fireEvent.click(
await screen.findByRole("button", { name: "Save and schedule restart" }),
);

await screen.findByText(
"Service restart command failed. Your current session is still active; retry restart.",
);
expect(
requests.filter((request) => request.url === "/api/status"),
).toHaveLength(2);
expect(setAuth).not.toHaveBeenCalled();
expect(routerMocks.navigate).not.toHaveBeenCalled();
});

test("keeps the success message after the restart command completes", async () => {
installApi({
passwordChange: true,
warnings: ["password_change"],
restartStatusSequence: ["command_completed", "command_failed"],
});
render(<EditorHarness initialSection="api" />);

fireEvent.change(await screen.findByLabelText("Password"), {
target: { value: "new-password" },
});
fireEvent.click(screen.getByRole("button", { name: "Save" }));
fireEvent.click(
await screen.findByRole("button", { name: "Save and schedule restart" }),
);

await screen.findByText("Configuration saved. Restart required.");
await act(
() =>
new Promise((resolve) => {
window.setTimeout(resolve, 600);
}),
);

expect(
screen.getByText("Configuration saved. Restart required."),
).toBeTruthy();
expect(
screen.queryByText(
"Service restart command failed. Your current session is still active; retry restart.",
),
).toBeNull();
});

test("uses singular category copy for one changed section", async () => {
Expand Down
6 changes: 5 additions & 1 deletion frontend/src/lib/config-api.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { apiFetch, apiRequest } from "#/lib/api";
import type { AppConfig } from "#/lib/config-model";
import type { AppConfig, StatusResponse } from "#/lib/config-model";

export type ConfigDocument = {
config: AppConfig;
Expand Down Expand Up @@ -91,3 +91,7 @@ export async function saveConfig(
export async function scheduleRestart(): Promise<void> {
await apiFetch("/api/service/restart", { method: "POST" });
}

export async function loadServiceStatus(): Promise<StatusResponse> {
return apiFetch<StatusResponse>("/api/status");
}
1 change: 1 addition & 0 deletions frontend/src/lib/config-model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,4 +65,5 @@ export type StatusResponse = {
api_bind: string;
api_port: number;
database_path: string;
restart_status: "idle" | "scheduled" | "command_completed" | "command_failed";
};
8 changes: 6 additions & 2 deletions frontend/src/locales/en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,10 @@ export const en = {
restartScheduled:
"Restart scheduled. The dashboard may disconnect briefly.",
restartFailed: "Restart failed: {{message}}",
restartCommandFailed:
"Service restart command failed. Your current session is still active; retry restart.",
restartStatusUnavailable:
"Unable to confirm the service restart. Your current session is still active; retry restart.",
},
restartDialog: {
title: "Schedule service restart?",
Expand Down Expand Up @@ -487,7 +491,7 @@ export const en = {
},
warnings: {
passwordChange:
"All sessions will be signed out after Save + Restart is scheduled.",
"Current sessions stay active until the service restarts, then all sessions are signed out.",
apiDisable: "The dashboard will be unavailable after restart.",
apiEndpointChange: "The dashboard address may change after restart.",
trustedProxiesChange:
Expand Down Expand Up @@ -564,7 +568,7 @@ export const en = {
"Also listen on a safe IPv6 companion address when one can be inferred.",
password: "Password",
passwordDescription:
"Changing this value saves and schedules restart in one step, then signs out every session.",
"Changing this value saves and schedules restart in one step. Sessions are signed out only after the new service starts.",
databasePath: "Database path",
},
timeouts: {
Expand Down
8 changes: 6 additions & 2 deletions frontend/src/locales/es.ts
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,10 @@ export const es = {
restartScheduled:
"Reinicio programado. El panel puede desconectarse brevemente.",
restartFailed: "Reinicio fallido: {{message}}",
restartCommandFailed:
"El comando de reinicio del servicio falló. La sesión actual sigue activa; vuelve a intentar el reinicio.",
restartStatusUnavailable:
"No se pudo confirmar el reinicio del servicio. La sesión actual sigue activa; vuelve a intentarlo.",
},
restartDialog: {
title: "¿Programar reinicio del servicio?",
Expand Down Expand Up @@ -505,7 +509,7 @@ export const es = {
},
warnings: {
passwordChange:
"Todas las sesiones se cerrarán tras programar Guardar + Reiniciar.",
"Las sesiones actuales siguen activas hasta que el servicio se reinicie; después se cerrarán todas.",
apiDisable: "El panel no estará disponible tras el reinicio.",
apiEndpointChange:
"La dirección del panel puede cambiar tras el reinicio.",
Expand Down Expand Up @@ -583,7 +587,7 @@ export const es = {
"Escuchar también en una dirección IPv6 complementaria segura cuando se pueda inferir una.",
password: "Contraseña",
passwordDescription:
"Cambiar este valor guarda y programa el reinicio en un solo paso y luego cierra todas las sesiones.",
"Cambiar este valor guarda y programa el reinicio en un solo paso. Las sesiones se cierran solo cuando inicia el nuevo servicio.",
databasePath: "Ruta de la base de datos",
},
timeouts: {
Expand Down
Loading
Loading