diff --git a/src/components/ops/channels-panel.test.ts b/src/components/ops/channels-panel.test.ts new file mode 100644 index 0000000..416c6e4 --- /dev/null +++ b/src/components/ops/channels-panel.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, it } from "vitest"; +import { allowlistDrift } from "./channels-panel"; + +describe("allowlistDrift", () => { + it("is silent when the server matches what the editor was seeded from", () => { + expect(allowlistDrift(["alice", "bob"], ["alice", "bob"], ["alice"])).toBeNull(); + // Order is not drift. + expect(allowlistDrift(["alice", "bob"], ["bob", "alice"], [])).toBeNull(); + }); + + it("names someone who self-onboarded after the panel loaded", () => { + // `carol` sent /claim while this panel sat open. Saving the box as typed + // replaces the whole list and removes her, with nothing on screen saying so. + const d = allowlistDrift(["alice"], ["alice", "carol"], ["alice"]); + expect(d).not.toBeNull(); + expect(d?.wouldRevoke).toEqual(["carol"]); + }); + + it("does not claim a revocation the operator already typed back in", () => { + const d = allowlistDrift(["alice"], ["alice", "carol"], ["alice", "carol"]); + expect(d).not.toBeNull(); + expect(d?.wouldRevoke).toEqual([]); + }); + + it("reports entries the server dropped as well", () => { + const d = allowlistDrift(["alice", "bob"], ["alice"], ["alice", "bob"]); + expect(d?.alsoChanged).toEqual(["bob"]); + }); +}); diff --git a/src/components/ops/channels-panel.tsx b/src/components/ops/channels-panel.tsx index 3cfba7f..5027406 100644 --- a/src/components/ops/channels-panel.tsx +++ b/src/components/ops/channels-panel.tsx @@ -3,6 +3,7 @@ import * as React from "react"; import { api } from "@/lib/api"; import { useAsync } from "@/hooks/use-async"; +import { useGatewayStatus } from "@/hooks/use-gateway-status"; import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; import { Badge } from "@/components/ui/badge"; import { Button } from "@/components/ui/button"; @@ -39,20 +40,82 @@ function telegramAllowlist(config: Record | null): string[] { return Array.isArray(allowed) ? (allowed as string[]) : []; } +/** + * What saving `next` would do to a server list that has moved since the editor + * was seeded from `seeded`. + * + * The POST replaces the allowlist wholesale, so anyone who self-onboarded via + * `/claim` after the panel loaded is silently revoked. The backend deliberately + * re-reads the freshest config under a lock to avoid clobbering them; the + * console defeated that by sending a stale snapshot back. + * + * Returns `null` when the server matches what the editor was seeded from — + * nothing to warn about. + */ +export function allowlistDrift( + seeded: string[], + server: string[], + next: string[], +): { wouldRevoke: string[]; alsoChanged: string[] } | null { + const seededSet = new Set(seeded); + const serverSet = new Set(server); + const addedOnServer = server.filter((u) => !seededSet.has(u)); + const goneFromServer = seeded.filter((u) => !serverSet.has(u)); + if (addedOnServer.length === 0 && goneFromServer.length === 0) return null; + const nextSet = new Set(next); + return { + // Only the ones the operator's box does NOT already carry: an entry they + // typed back in is not being revoked. + wouldRevoke: addedOnServer.filter((u) => !nextSet.has(u)), + alsoChanged: goneFromServer, + }; +} + export function ChannelsPanel() { - const { data, loading, error, refresh } = useAsync(() => api.channels(), []); + const { data, loading, error, refresh, loaded } = useAsync(() => api.channels(), []); const cfg = useAsync(() => api.config(), []); const tgConnected = !!data?.configured.includes("telegram"); + const gateway = useGatewayStatus(); + // Set while the gateway is reloading because of a save we just made, so the + // panel can say so instead of presenting the outage as a load error. + const [reloading, setReloading] = React.useState(false); + const settleTimer = React.useRef | null>(null); - const refreshNow = () => { + const refreshNow = React.useCallback(() => { refresh(); cfg.refresh(); - }; + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [refresh, cfg.refresh]); + // Channel config changes reload the runtime (a few seconds), so refetch after a - // short settle delay — an instant refetch would race the gateway restart. - const refreshAfterReload = () => { - setTimeout(refreshNow, 3000); - }; + // short settle delay — an instant refetch would race the gateway restart. The + // timer is held in a ref and cleared on unmount; it used to be a bare + // `setTimeout` that fired into an unmounted tree. + const refreshAfterReload = React.useCallback(() => { + setReloading(true); + if (settleTimer.current) clearTimeout(settleTimer.current); + settleTimer.current = setTimeout(refreshNow, 3000); + }, [refreshNow]); + + React.useEffect( + () => () => { + if (settleTimer.current) clearTimeout(settleTimer.current); + }, + [], + ); + + // Leave the reloading state when the gateway answers again — and give up + // after a bounded window rather than spinning forever, naming the recovery. + React.useEffect(() => { + if (!reloading) return undefined; + if (gateway.connection === "online") { + setReloading(false); + refreshNow(); + return undefined; + } + const giveUp = setTimeout(() => setReloading(false), 60_000); + return () => clearTimeout(giveUp); + }, [reloading, gateway.connection, refreshNow]); return (
@@ -62,9 +125,17 @@ export function ChannelsPanel() { {/* Gate on the config fetch too: the allowlist editor is seeded from GET /config, so rendering it before config loads (or after it fails) would let "Save allowlist" persist an empty deny-all list. */} + {reloading && ( +
+ Reloading the runtime after your change… the panel keeps its content and + refreshes on its own. If it does not come back, run{" "} + systemctl --user reset-failed rantaiclaw.service and start it again. +
+ )} (null); // Prefill the allowlist editor with the saved list once connected. const savedAllowlist = allowedUsers.join(", "); @@ -118,9 +195,13 @@ function TelegramCard({ .map((s) => s.trim()) .filter(Boolean); + // Both, not either. The gateway sets `warning` when the allowlist is empty or + // contains `*`, and puts the restart notice in `note` — so the `else if` here + // suppressed the restart notice in exactly the two states an operator is most + // likely to be in while editing. const notify = (r: { warning?: string | null; note?: string }) => { if (r.warning) toast.warning(r.warning); - else if (r.note) toast.message(r.note); + if (r.note) toast.message(r.note); }; const connect = async () => { @@ -140,11 +221,19 @@ function TelegramCard({ } }; - const saveAllowlist = async () => { + // The POST replaces the list wholesale, and the editor is seeded from a + // snapshot taken when the panel loaded — so anyone who self-onboarded via + // `/claim` since then is silently revoked. The backend goes out of its way to + // avoid clobbering that (it re-reads the freshest config under a lock); this + // makes the console stop defeating it, by showing the operator the removal and + // asking first. + const runSave = async () => { setBusy(true); try { const r = await api.updateTelegramAllowlist(parseUsers()); - toast.success("Allowlist updated"); + // What the SERVER stored, not what was requested. A mismatch between the + // two is exactly what an operator needs to see. + toast.success(`Allowlist updated — ${r.allowed_users} sender(s) allowed`); notify(r); onReload(); } catch (e) { @@ -154,6 +243,31 @@ function TelegramCard({ } }; + const saveAllowlist = async () => { + setBusy(true); + let fresh: string[] | null = null; + try { + fresh = telegramAllowlist(await api.config()); + } catch { + // A failed pre-check must not block the save — it is a courtesy, not a + // gate. Falling through means the operator gets the old behaviour, which + // is what they would have had anyway. + fresh = null; + } finally { + setBusy(false); + } + + if (fresh) { + const d = allowlistDrift(allowedUsers, fresh, parseUsers()); + if (d) { + setDrift(d); + return; + } + } + + await runSave(); + }; + const disconnect = async () => { setBusy(true); try { @@ -255,6 +369,32 @@ function TelegramCard({ busy={busy} onConfirm={disconnect} /> + setDrift(null)} + title="The allowlist changed while this was open" + description={ + drift + ? [ + drift.wouldRevoke.length > 0 + ? `Saving now removes: ${drift.wouldRevoke.join(", ")} — added on the server since this panel loaded (a /claim or /bind, most likely).` + : "", + drift.alsoChanged.length > 0 + ? `Already removed on the server: ${drift.alsoChanged.join(", ")}.` + : "", + "Save anyway to replace the server's list with what is in the box.", + ] + .filter(Boolean) + .join(" ") + : "" + } + confirmLabel="Save anyway" + busy={busy} + onConfirm={async () => { + setDrift(null); + await runSave(); + }} + /> ); } diff --git a/src/components/ops/shared.tsx b/src/components/ops/shared.tsx index 9ffc261..ce28511 100644 --- a/src/components/ops/shared.tsx +++ b/src/components/ops/shared.tsx @@ -64,12 +64,23 @@ export function PanelFrame({ error, empty, onRefresh, + loaded, children, }: { loading?: boolean; error?: string | null; empty?: boolean; onRefresh?: () => void; + /** + * Whether this panel has ever successfully loaded. + * + * With it, a REFRESH failure keeps the content on screen and shows the error + * as a non-blocking strip; without it (still the default for callers that do + * not pass it) any error blanked the whole panel — which made the most likely + * outcome of a *successful* save an error screen, indistinguishable to the + * operator from the save having failed. + */ + loaded?: boolean; children: React.ReactNode; }) { if (loading) { @@ -79,6 +90,23 @@ export function PanelFrame({
); } + if (error && loaded) { + // Refresh failure: keep what is on screen, say what went wrong. + return ( + <> +
+ + {error} + {onRefresh && ( + + )} +
+ {children} + + ); + } if (error) { return ( (fn: () => Promise, deps: React.DependencyList = [ const [loading, setLoading] = React.useState(true); const [refreshing, setRefreshing] = React.useState(false); const loaded = React.useRef(false); + // Request token: an older in-flight response must not overwrite a newer one. + // The case an operator produces is hitting Refresh during a gateway restart — + // the slow pre-restart response would land last and show pre-save data. + const reqId = React.useRef(0); // Blank to the loading state only on the first load or a deps change; a manual // refresh keeps the stale content mounted (via `refreshing`) so the panel // doesn't flash and lose scroll position on every poll. const run = React.useCallback(async (isRefresh: boolean) => { + const id = ++reqId.current; if (isRefresh && loaded.current) setRefreshing(true); else setLoading(true); setError(null); try { - setData(await fn()); + const r = await fn(); + if (id !== reqId.current) return; + setData(r); loaded.current = true; } catch (e) { + if (id !== reqId.current) return; setError(e instanceof Error ? e.message : String(e)); } finally { - setLoading(false); - setRefreshing(false); + if (id === reqId.current) { + setLoading(false); + setRefreshing(false); + } } // eslint-disable-next-line react-hooks/exhaustive-deps }, deps); @@ -34,5 +44,9 @@ export function useAsync(fn: () => Promise, deps: React.DependencyList = [ const refresh = React.useCallback(() => run(true), [run]); - return { data, error, loading, refreshing, refresh }; + // `loaded` distinguishes an INITIAL-load failure (no data — the error state is + // right) from a REFRESH failure (data is already on screen — keep it). Without + // it, `PanelFrame` blanked the whole panel whenever a refresh failed, so the + // most likely outcome of a *successful* save was an error screen. + return { data, error, loading, refreshing, loaded: loaded.current, refresh }; }