Enable no-floating-promises and make fire-and-forget explicit

.kiro/skills/typescript/SKILL.md

@@ -12,6 +12,16 @@ description: TypeScript conventions and rules for Graph Explorer, including bran
 - Every commit should have no type errors
 - Use explicit type aliases instead of `ReturnType<typeof ...>` when available (e.g., use `AppStore` instead of `ReturnType<typeof getAppStore>`)
 
+## Floating Promises
+
+`typescript/no-floating-promises` is enabled as an error, so every unhandled promise must be marked explicitly. Pick the idiom by meaning:
+
+- **`.catch(logAndNotify(message, options?))`** (`logAndNotify` from `@/utils`) — when the user should know about the failure. Logs via `logger.warn` and shows a warning toast with `message`. This is the idiom for persisted-atom writes from a user action (saving a connection, editing a style, toggling a setting). Build the `message` with `useTranslations` when it references node/edge vocabulary so it reads correctly per query engine; pass `options` for sonner extras like `description`.
+- **`.catch(logAndIgnore)`** (`logAndIgnore` from `@/utils`) — when a rejection is worth logging but not worth interrupting the user (e.g. a background canvas icon fetch). Logs via `logger.warn` only (warn, not error, since an ignored rejection is non-blocking).
+- **bare `void expr;`** — when the rejection is genuinely inert or already handled elsewhere: aborted `navigate()`, a refetch whose errors surface through query state, a self-catching async helper, and test-state seeding.
+
+See ADR `docs/adr/20260618-explicit-floating-promise-convention.md` for the full rationale.
+
 ## Branded Types
 
 The project uses branded types from `@/utils` for type safety. These prevent accidental mixing of similar types at compile time.

.oxlintrc.json

@@ -28,7 +28,7 @@
     "jest/require-to-throw-message": "off",
     "jest/valid-expect": "off",
     "typescript/unbound-method": "off",
-    "typescript/no-floating-promises": "off",
+    "typescript/no-floating-promises": "error",
     "no-unused-vars": [
       "error",
       {

docs/adr/20260616-indexeddb-not-localstorage-for-persistence.md

@@ -2,7 +2,7 @@
 
 - **Status:** Accepted
 - **Date:** 2026-06-16
-- **Related:** ADR `per-key-diff-merge-cross-tab-reconciliation` builds the cross-tab fix on top of this constraint. Issue #1820.
+- **Related:** ADR `per-key-diff-merge-cross-tab-reconciliation` builds the cross-tab fix on top of this constraint. ADR `explicit-floating-promise-convention` makes this write path's fire-and-forget setters explicit. Issue #1820.
 
 ## Context
 

docs/adr/20260616-per-key-diff-merge-cross-tab-reconciliation.md

@@ -2,7 +2,7 @@
 
 - **Status:** Accepted
 - **Date:** 2026-06-16
-- **Related:** ADR `indexeddb-not-localstorage-for-persistence` (IndexedDB constraint) is the storage substrate this reconciliation runs on. Issue #1820 (the clobber bug); inverse of #1788 / per-tab active connection, which wants per-tab _divergence_ rather than reconciliation.
+- **Related:** ADR `indexeddb-not-localstorage-for-persistence` (IndexedDB constraint) is the storage substrate this reconciliation runs on. ADR `explicit-floating-promise-convention` makes the fire-and-forget setters on this write path explicit. Issue #1820 (the clobber bug); inverse of #1788 / per-tab active connection, which wants per-tab _divergence_ rather than reconciliation.
 
 ## Context
 

docs/adr/20260618-explicit-floating-promise-convention.md

@@ -0,0 +1,33 @@
+# ADR — Explicit handling of intentional floating promises
+
+- **Status:** Accepted
+- **Date:** 2026-06-18
+- **Related:** ADR `per-key-diff-merge-cross-tab-reconciliation` and `indexeddb-not-localstorage-for-persistence` describe the persistence write path whose fire-and-forget nature motivates this convention. Enables the `typescript/no-floating-promises` lint rule.
+
+## Context
+
+Many call sites start a promise and never await it. The most common is a write to a localForage-backed Jotai atom (`atomWithLocalForage`): the in-memory value updates synchronously, while the durable write happens in the background and is "never awaited in production" (see the write-path comment in `core/StateProvider/atomWithLocalForage.ts`). Other cases include react-router `navigate()`, fire-and-forget query refetches, and test-state seeding.
+
+Until now `typescript/no-floating-promises` was off, so these reads as ordinary statements. The hazard: a genuinely-swallowed persistence failure looks identical to an intentional fire-and-forget. The user's hypothesis was that some of those silent `store.set(...)` calls were latent bugs hiding behind that ambiguity.
+
+## Decision
+
+Turn on `typescript/no-floating-promises` (`error`) so every unhandled promise must be **explicitly** marked. Use one of three idioms by meaning:
+
+1. **`.catch(logAndNotify(message, options?))`** (`logAndNotify` from `@/utils`) — builds a `.catch` handler that logs the rejection via `logger.warn` **and** shows the user a warning toast. Use when an unawaited failure is something the **user should know about** — most persisted-atom writes from a user action (saving a connection, editing a style, toggling a setting). Each site passes its own message; query-language vocabulary (node/edge) is resolved with `useTranslations` at the call site so the toast reads correctly per engine (e.g. "resource" for SPARQL). `options` forwards sonner options such as `description`.
+
+2. **`.catch(logAndIgnore)`** (`logAndIgnore` from `@/utils`) — a `.catch` handler that only logs the rejection via `logger.warn`, no toast. Use when a rejection is **meaningful enough to log but not worth interrupting the user** — e.g. a background canvas icon fetch. (Warn, not error: an explicitly-ignored rejection is non-blocking by construction.)
+
+3. **bare `void expr;`** — use when a rejection is **genuinely inert** or already handled elsewhere: aborted `navigate()`, a refetch whose errors surface through query state, a self-catching async helper, and **test-state seeding** (a test seeding store state does not care whether the value lands in durable storage).
+
+Both `logAndNotify` and `logAndIgnore` log at `warn`; they differ only in whether the user is notified. The broader reconciliation contract for persistence errors (reject + per-key merge, per `per-key-diff-merge-cross-tab-reconciliation`) is still a separate effort; `logAndNotify` surfaces the failure but does not yet reconcile or retry the write.
+
+### Alternatives considered
+
+**Centralize the `.catch` inside `atomWithLocalForage` and return `void`.** The writer atom could attach its own `.catch(logAndIgnore)` and return `void`, deleting most of the ~25 `set(...).catch(logAndIgnore)` handlers at call sites. We rejected this for now because the in-flight persistence work (reject + toast, per `per-key-diff-merge-cross-tab-reconciliation`) needs _per-call-site_ control over how a failure is surfaced — a single swallow point in the writer would have to be unwound again to differentiate "log only" from "toast the user." Keeping the decision at each call site also keeps the persisted-write sites **greppable** for the #1854 audit. Once the surfacing contract is settled, collapsing the common case back into the writer is a reasonable follow-up.
+
+## Consequences
+
+- A swallowed persistence error is now either surfaced to the user (`.catch(logAndNotify(...))`), logged (`.catch(logAndIgnore)`), or a deliberate, reviewed `void` — no longer invisible.
+- Reviewers have a clear rule: user should know → `logAndNotify`; log-only → `logAndIgnore`; inert → `void`.
+- The lint rule prevents silent fire-and-forget from being reintroduced anywhere in the codebase.

packages/graph-explorer/src/components/Button/NavButton.tsx

@@ -24,7 +24,7 @@ function NavButton({ to, navOptions, ...props }: NavButtonProps) {
   const navigate = useNavigate();
 
   function handleClick() {
-    navigate(to, navOptions);
+    void navigate(to, navOptions);
   }
 
   return <Button {...props} onClick={handleClick} />;

packages/graph-explorer/src/components/Redirect.tsx

@@ -4,7 +4,7 @@ import { useNavigate } from "react-router";
 const Redirect = ({ to }: { to: string }) => {
   const navigate = useNavigate();
   useEffect(() => {
-    navigate(to, {
+    void navigate(to, {
       replace: true,
     });
   }, [navigate, to]);

packages/graph-explorer/src/components/utils/canvas/drawImage.ts

@@ -1,4 +1,4 @@
-import { env } from "@/utils";
+import { env, logAndIgnore } from "@/utils";
 
 import type { BoundingBox } from "./types";
 
@@ -104,21 +104,23 @@ const drawImage = (
 
   if (src.match(/\.svg$/i)) {
     IMAGES_PENDING.add(`${src}::${color || "default"}`);
-    fetchIcon(src).then(icon => {
-      IMAGES_PENDING.delete(`${src}::${color || "default"}`);
-
-      let iconSrc = icon;
-      if (icon && color) {
-        iconSrc = colorizeSvg(icon, color);
-      }
-
-      if (!iconSrc) {
-        return;
-      }
-
-      createImage(iconSrc);
-      context.restore();
-    });
+    fetchIcon(src)
+      .then(icon => {
+        IMAGES_PENDING.delete(`${src}::${color || "default"}`);
+
+        let iconSrc = icon;
+        if (icon && color) {
+          iconSrc = colorizeSvg(icon, color);
+        }
+
+        if (!iconSrc) {
+          return;
+        }
+
+        createImage(iconSrc);
+        context.restore();
+      })
+      .catch(logAndIgnore);
 
     return;
   }

packages/graph-explorer/src/connector/LoggerConnector.test.ts

@@ -1,3 +1,7 @@
+import { toast } from "sonner";
+
+import { logger } from "@/utils";
+
 import {
   ClientLoggerConnector,
   ServerLoggerConnector,
@@ -15,53 +19,70 @@ describe("ClientLoggerConnector", () => {
 });
 
 describe("ServerLoggerConnector", () => {
-  test("should send logs to the server", () => {
+  test("should send logs to the server", async () => {
     const mockFetch = vi.fn().mockResolvedValue({});
     vi.stubGlobal("fetch", mockFetch);
 
     const connector = new ServerLoggerConnector("https://example.com/");
 
-    connector.error("error msg");
+    await connector.error("error msg");
     expect(mockFetch).toHaveBeenCalledWith("https://example.com/logger", {
       method: "POST",
       headers: { level: "error", message: JSON.stringify("error msg") },
     });
 
-    connector.warn("warn msg");
+    await connector.warn("warn msg");
     expect(mockFetch).toHaveBeenCalledWith("https://example.com/logger", {
       method: "POST",
       headers: { level: "warn", message: JSON.stringify("warn msg") },
     });
 
-    connector.info("info msg");
+    await connector.info("info msg");
     expect(mockFetch).toHaveBeenCalledWith("https://example.com/logger", {
       method: "POST",
       headers: { level: "info", message: JSON.stringify("info msg") },
     });
 
-    connector.debug("debug msg");
+    await connector.debug("debug msg");
     expect(mockFetch).toHaveBeenCalledWith("https://example.com/logger", {
       method: "POST",
       headers: { level: "debug", message: JSON.stringify("debug msg") },
     });
 
-    connector.trace("trace msg");
+    await connector.trace("trace msg");
     expect(mockFetch).toHaveBeenCalledWith("https://example.com/logger", {
       method: "POST",
       headers: { level: "trace", message: JSON.stringify("trace msg") },
     });
   });
 
-  test("should strip trailing slash from connection URL", () => {
+  test("should strip trailing slash from connection URL", async () => {
     const mockFetch = vi.fn().mockResolvedValue({});
     vi.stubGlobal("fetch", mockFetch);
 
     const connector = new ServerLoggerConnector("https://example.com/");
-    connector.info("test");
+    await connector.info("test");
 
     expect(mockFetch).toHaveBeenCalledWith(
       "https://example.com/logger",
       expect.any(Object),
     );
   });
+
+  test("should warn the user and console when sending a log to the server fails", async () => {
+    const failure = new Error("network down");
+    vi.stubGlobal("fetch", vi.fn().mockRejectedValue(failure));
+
+    const connector = new ServerLoggerConnector("https://example.com/");
+
+    await expect(connector.error("error msg")).resolves.toBeUndefined();
+
+    expect(vi.mocked(logger.warn)).toHaveBeenCalledWith(
+      "Failed to send log to server",
+      failure,
+    );
+    expect(vi.mocked(toast.warning)).toHaveBeenCalledWith(
+      "Failed to send log to the server.",
+    );
+  });
 });

packages/graph-explorer/src/connector/LoggerConnector.ts

@@ -1,13 +1,15 @@
+import { toast } from "sonner";
+
 import { logger } from "@/utils";
 
 export type LogLevel = "error" | "warn" | "info" | "debug" | "trace";
 
 export interface LoggerConnector {
-  error(message: unknown): void;
-  warn(message: unknown): void;
-  info(message: unknown): void;
-  debug(message: unknown): void;
-  trace(message: unknown): void;
+  error(message: unknown): Promise<void>;
+  warn(message: unknown): Promise<void>;
+  info(message: unknown): Promise<void>;
+  debug(message: unknown): Promise<void>;
+  trace(message: unknown): Promise<void>;
 }
 
 /** Sends log messages to the server in the connection configuration. */
@@ -22,56 +24,66 @@ export class ServerLoggerConnector implements LoggerConnector {
   }
 
   public error(message: unknown) {
-    this._clientLogger.error(message);
+    void this._clientLogger.error(message);
     return this._sendLog("error", message);
   }
 
   public warn(message: unknown) {
-    this._clientLogger.warn(message);
+    void this._clientLogger.warn(message);
     return this._sendLog("warn", message);
   }
 
   public info(message: unknown) {
-    this._clientLogger.info(message);
+    void this._clientLogger.info(message);
     return this._sendLog("info", message);
   }
 
   public debug(message: unknown) {
-    this._clientLogger.info(message);
+    void this._clientLogger.info(message);
     return this._sendLog("debug", message);
   }
 
   public trace(message: unknown) {
-    this._clientLogger.trace(message);
+    void this._clientLogger.trace(message);
     return this._sendLog("trace", message);
   }
 
-  private _sendLog(level: LogLevel, message: unknown) {
-    return fetch(this._baseUrl, {
-      method: "POST",
-      headers: {
-        level,
-        message: JSON.stringify(message),
-      },
-    }).catch(err => logger.error("Failed to send log to server", err));
+  private async _sendLog(level: LogLevel, message: unknown) {
+    try {
+      await fetch(this._baseUrl, {
+        method: "POST",
+        headers: {
+          level,
+          message: JSON.stringify(message),
+        },
+      });
+    } catch (error) {
+      logger.warn("Failed to send log to server", error);
+      toast.warning("Failed to send log to the server.");
+    }
   }
 }
 
 /** Sends logs to the browser's console. */
 export class ClientLoggerConnector implements LoggerConnector {
-  error(message: unknown): void {
+  error(message: unknown): Promise<void> {
     logger.error(message);
+    return Promise.resolve();
   }
-  warn(message: unknown): void {
+  warn(message: unknown): Promise<void> {
     logger.warn(message);
+    return Promise.resolve();
   }
-  info(message: unknown): void {
+  info(message: unknown): Promise<void> {
     logger.log(message);
+    return Promise.resolve();
   }
-  debug(message: unknown): void {
+  debug(message: unknown): Promise<void> {
     logger.debug(message);
+    return Promise.resolve();
   }
-  trace(message: unknown): void {
+  trace(message: unknown): Promise<void> {
     logger.log(message);
+    return Promise.resolve();
   }
 }