0.4.0 — fix queue-add race + per-item remove button (#10, #11)

2026-10-11 16:09:29 +00:00
parent 3287bcaca4
commit ef27b59762
14 changed files with 448 additions and 29 deletions
@@ -153,6 +153,26 @@ export default function App() {
  // never acts on a stale closure. Written each render (same pattern as sse.ts).
  const latestRef = useRef<{ game: GameState | null; goHome: () => void; loadGame: (id: string) => void } | null>(null);

+  // The single entry point for adopting a full game state — SSE `state`/`turn`
+  // events AND the REST responses (add/remove/jump) AND game loads all flow
+  // through here. Two rules:
+  //   • a *different* game id (new game, deep link, resume) is always adopted;
+  //   • the *same* game is adopted only when its `version` is not older than
+  //     the current state's ("never go backwards").
+  // The comparison happens inside the functional updater, so it always reads
+  // the latest committed state — never a stale closure. This is what makes a
+  // late-arriving stale SSE snapshot (e.g. the on-connect resync taken just
+  // before an add was saved, or an EventSource auto-reconnect) harmless: it
+  // carries a lower version than the fresher state the REST response already
+  // applied, so it is dropped instead of clobbering the queued item (#10).
+  // Legacy games saved before `version` existed are treated as version 0.
+  const applyState = useCallback((s: GameState) => {
+    setGame((cur) => {
+      if (!cur || cur.id !== s.id) return s;
+      return (s.version ?? 0) >= (cur.version ?? 0) ? s : cur;
+    });
+  }, []);
+
  // On the home screen: load the scenario manifest and the list of saved games
  // (newest-updated first) so the user can resume a prior game.
  useEffect(() => {
@@ -192,7 +212,7 @@ export default function App() {
        if (!alive) return;
        persistActiveGameId(id); // remember it so a future bare `/` load resumes it
        setActiveGameId(id);
-        setGame(game);
+        applyState(game);
        setScreen({ name: 'game' });
      } catch {
        if (!alive) return;
@@ -250,10 +270,9 @@ export default function App() {
  }, [game]);

  // Resync from the persisted server state on connect and after every turn.
-  // Handlers only call the stable setGame, so no stale-closure concerns.
  useSse(game?.id ?? null, {
-    onState: (s) => setGame(s),
-    onTurn: (s) => setGame(s),
+    onState: applyState,
+    onTurn: applyState,
  });

  const play = useCallback(async (presetId: string) => {
@@ -282,7 +301,7 @@ export default function App() {
        // Remember this as the active game so a refresh resumes it.
        persistActiveGameId(game.id);
        setActiveGameId(game.id);
-        setGame(game);
+        applyState(game);
        setAdvisors([]);
        setScreen({ name: 'game' });
      } catch (e) {
@@ -295,18 +314,20 @@ export default function App() {
  );

  // Add an action to the pending round. Fast, non-LLM — no `busy`; the pending
-  // list updates from the response and the SSE `state` event.
+  // list updates from the response and the SSE `state` event. Both are
+  // version-gated via applyState, so the item can never be clobbered by a
+  // stale snapshot (#10).
  const runAddAction = useCallback(
    async (text: string) => {
      if (!game) return;
      try {
        const { game: g } = await api.addAction(game.id, text);
-        setGame(g);
+        applyState(g);
      } catch (e) {
        toastError(e);
      }
    },
-    [game],
+    [game, applyState],
  );

  // Add a diplomatic message to the pending round. Fast, non-LLM — no `busy`.
@@ -315,12 +336,27 @@ export default function App() {
      if (!game) return;
      try {
        const { game: g } = await api.addDiplomacy(game.id, to, message);
-        setGame(g);
+        applyState(g);
      } catch (e) {
        toastError(e);
      }
    },
-    [game],
+    [game, applyState],
+  );
+
+  // Remove a queued item by index. Fast, non-LLM — no `busy`; the pending list
+  // updates from the response and the SSE `state` event (version-gated).
+  const runRemoveQueueItem = useCallback(
+    async (index: number) => {
+      if (!game) return;
+      try {
+        const { game: g } = await api.removeQueueItem(game.id, index);
+        applyState(g);
+      } catch (e) {
+        toastError(e);
+      }
+    },
+    [game, applyState],
  );

  // Jump forward: the single LLM call that commits the pending round. Only this
@@ -330,13 +366,13 @@ export default function App() {
    setBusy(true);
    try {
      const { state } = await api.jump(game.id);
-      setGame(state);
+      applyState(state);
    } catch (e) {
      toastError(e);
    } finally {
      setBusy(false);
    }
-  }, [game]);
+  }, [game, applyState]);

  const askAdvisor = useCallback(
    async (prompt?: string) => {
@@ -362,7 +398,7 @@ export default function App() {
      // Make this the active game so a bare `/` reload keeps resuming it.
      persistActiveGameId(id);
      setActiveGameId(id);
-      setGame(game);
+      applyState(game);
      setAdvisors([]);
      setScreen({ name: 'game' });
    } catch (e) {
@@ -401,6 +437,7 @@ export default function App() {
          advisors={advisors}
          onAddAction={runAddAction}
          onAddDiplomacy={runAddDiplomacy}
+          onRemoveQueueItem={runRemoveQueueItem}
          onAdvisor={askAdvisor}
          onJump={runJump}
          onHome={goHome}
@@ -440,6 +477,7 @@ interface GameScreenProps {
  advisors: string[];
  onAddAction: (a: string) => void;
  onAddDiplomacy: (to: string, m: string) => void;
+  onRemoveQueueItem: (index: number) => void;
  onAdvisor: (p?: string) => void;
  onJump: () => void;
  onHome: () => void;
@@ -463,6 +501,7 @@ function GameScreen({
  advisors,
  onAddAction,
  onAddDiplomacy,
+  onRemoveQueueItem,
  onAdvisor,
  onJump,
  onHome,
@@ -663,6 +702,7 @@ function GameScreen({
          advisors={advisors}
          onAddAction={onAddAction}
          onAddDiplomacy={onAddDiplomacy}
+          onRemoveQueueItem={onRemoveQueueItem}
          onAdvisor={onAdvisor}
          onJump={onJump}
          inWindow
@@ -1,13 +1,21 @@
 import { useState } from 'react';
-import { Loader2, MessageSquare, Radar, Zap } from 'lucide-react';
+import { Loader2, MessageSquare, Radar, Trash2, Zap } from 'lucide-react';
 import { Badge } from './ui/badge';
 import { Button } from './ui/button';
 import { Card, CardContent, CardHeader, CardTitle } from './ui/card';
+import {
+  Dialog,
+  DialogContent,
+  DialogDescription,
+  DialogFooter,
+  DialogHeader,
+  DialogTitle,
+} from './ui/dialog';
 import { Input } from './ui/input';
 import { Textarea } from './ui/textarea';
 import { Tabs, TabsContent, TabsList, TabsTrigger } from './ui/tabs';
 import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from './ui/select';
-import type { GameState } from '@shared/types';
+import type { GameState, RoundItem } from '@shared/types';

 interface Props {
  game: GameState;
@@ -19,6 +27,8 @@ interface Props {
  onAddAction: (text: string) => void;
  /** Queue a diplomatic message onto the pending round (no turn advance). */
  onAddDiplomacy: (to: string, message: string) => void;
+  /** Remove one queued item by its index in `game.pending` (no turn advance). */
+  onRemoveQueueItem: (index: number) => void;
  onAdvisor: (prompt?: string) => void;
  /** Commit the whole pending round in a single LLM turn. */
  onJump: () => void;
@@ -29,13 +39,22 @@ interface Props {
  inWindow?: boolean;
 }

+/** One-line, display-safe rendering of a queued item (text truncated). */
+function describeItem(item: RoundItem, nationName: (id?: string) => string): string {
+  const who = item.kind === 'diplomacy' ? ` to ${nationName(item.to)}` : '';
+  const text = item.text.length > 72 ? `${item.text.slice(0, 71)}…` : item.text;
+  return `${who ? `${who}:` : 'Action:'} “${text}”`;
+}
+
 /**
 * The player's command deck ("Your move"). Three tabs — Action, Diplomacy,
 * Advisor — each with a small "Add" button that queues an item onto the pending
 * round (these never advance the turn). Below the tabs a visible pending-queue
- * list and one prominent "Jump forward" button commit the whole round in a
- * single LLM turn. While a jump is in flight the commit is disabled and a hint
- * is shown; an empty jump (nothing queued) is allowed and advances time.
+ * list — each row has a small trash button that removes that item (with a
+ * confirmation dialog) — and one prominent "Jump forward" button that commits
+ * the whole round in a single LLM turn. While a jump is in flight the commit
+ * is disabled (as is queue editing) and a hint is shown; an empty jump
+ * (nothing queued) is allowed and advances time.
 */
 export default function ActionPanel({
  game,
@@ -43,6 +62,7 @@ export default function ActionPanel({
  advisors,
  onAddAction,
  onAddDiplomacy,
+  onRemoveQueueItem,
  onAdvisor,
  onJump,
  inWindow = false,
@@ -52,6 +72,8 @@ export default function ActionPanel({
  const [target, setTarget] = useState('');
  const [message, setMessage] = useState('');
  const [prompt, setPrompt] = useState('');
+  // Index of the queued item the user is about to remove, or null (dialog closed).
+  const [confirmRemoveIndex, setConfirmRemoveIndex] = useState<number | null>(null);

  const others = Object.entries(game.nations)
    .filter(([id]) => id !== game.playerNationId)
@@ -79,6 +101,10 @@ export default function ActionPanel({
  };

  const pending = game.pending;
+  // The item the confirmation dialog is about (re-derived from the live list,
+  // so if the queue changes while the dialog is open it tracks the current
+  // item at that index — or the dialog closes if the index went out of range).
+  const removeTarget = confirmRemoveIndex !== null ? pending[confirmRemoveIndex] : undefined;
  const queuedLabel = busy
    ? 'Simulating…'
    : `Jump forward${pending.length > 0 ? ` · ${pending.length} queued` : ''}`;
@@ -227,6 +253,18 @@ export default function ActionPanel({
                      </span>
                    )}
                    <span className="min-w-0 flex-1 truncate text-foreground/85">{item.text}</span>
+                    <Button
+                      variant="ghost"
+                      size="icon-sm"
+                      className="shrink-0 text-muted-foreground hover:text-destructive"
+                      onClick={() => setConfirmRemoveIndex(i)}
+                      disabled={busy}
+                      title="Remove from this round"
+                      aria-label="Remove this queued item"
+                      data-testid="remove-queue-item"
+                    >
+                      <Trash2 className="size-3.5" />
+                    </Button>
                  </li>
                ))}
              </ul>
@@ -234,6 +272,39 @@ export default function ActionPanel({
          )}
        </div>

+        {/* Confirmation for removing a queued item (never removes without asking). */}
+        <Dialog open={removeTarget !== undefined} onOpenChange={(open) => !open && setConfirmRemoveIndex(null)}>
+          <DialogContent className="max-w-sm" data-testid="remove-confirm-dialog">
+            <DialogHeader>
+              <DialogTitle>Remove queued item?</DialogTitle>
+              <DialogDescription>
+                {removeTarget ? (
+                  <>
+                    {describeItem(removeTarget, nationName)} will be removed from this round and
+                    won’t be simulated on the next jump.
+                  </>
+                ) : null}
+              </DialogDescription>
+            </DialogHeader>
+            <DialogFooter>
+              <Button variant="outline" onClick={() => setConfirmRemoveIndex(null)} data-testid="remove-cancel">
+                Cancel
+              </Button>
+              <Button
+                variant="destructive"
+                onClick={() => {
+                  if (confirmRemoveIndex !== null) onRemoveQueueItem(confirmRemoveIndex);
+                  setConfirmRemoveIndex(null);
+                }}
+                data-testid="remove-confirm"
+              >
+                <Trash2 className="size-4" />
+                Remove
+              </Button>
+            </DialogFooter>
+          </DialogContent>
+        </Dialog>
+
        <Button className="h-11 w-full" onClick={onJump} disabled={busy} data-testid="jump-forward">
          {busy ? <Loader2 className="size-4 animate-spin" /> : <Radar className="size-4" />}
          {queuedLabel}
@@ -8,6 +8,7 @@ import type {
  AdvisorResponse,
  CreateGameResponse,
  GameSummary,
+  RemoveQueueItemResponse,
  TurnResponse,
 } from '@shared/protocol';

@@ -48,6 +49,13 @@ export const api = {
      method: 'POST',
      body: JSON.stringify({ to, message }),
    }),
+  // Remove one queued item from the pending round, by its index in `game.pending`.
+  // Fast, non-LLM — no turn advance. Returns the updated game.
+  removeQueueItem: (id: string, index: number) =>
+    req<RemoveQueueItemResponse>(`/api/games/${id}/queue/remove`, {
+      method: 'POST',
+      body: JSON.stringify({ index }),
+    }),
  // Commit the whole pending round (or an empty one) in a single LLM turn.
  jump: (id: string) =>
    req<TurnResponse>(`/api/games/${id}/jump`, {