Add undoable rollback/redo (Phase 2)

Rolling back is now non-destructive: instead of truncating the future,
it restores the target checkpoint's snapshot and points
Game.rollback_head_checkpoint_id at it. Later checkpoints/events are
kept and rendered greyed; rolling forward to one redoes the rollback
(same endpoint). The abandoned future is discarded only when a new
action is taken from a rolled-back state -- maintain_rollback_history
truncates it, clears the head, and bumps rollback_timeline_version,
now the sole trigger for the frontend log reset.

- Game.rollback_head_checkpoint_id (nullable; in SNAPSHOT_EXCLUDE) +
  Alembic migration.
- crud_rollback: rollback_to_checkpoint sets the head (None at latest);
  _discard_future + head-clear + version-bump moved into the next scene
  action's capture.
- EventLog.svelte: greyed future, redo (forward) affordance, a "rolled
  back" banner; dropped the now-misleading destructive confirm.
- Tests rewritten for head-pointer semantics (+ redo, action-after-
  rollback truncation, scene-end head clear); HTTP integration test
  extended to drive the full cycle over the wire.

Verified end-to-end in a browser: rollback greys the future and reverts
live state (captaincy/ranks), redo restores it, no errors.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

TODO.md

@@ -1,5 +1,5 @@
 ## Major features
-- [~] **Rollback of game state.** *Phase 1 implemented* (full-state JSON snapshots in a `Checkpoint` table, captured per action by the broadcast middleware, restored by `crud_rollback.rollback_to_checkpoint`; scene-confined; server-enforced for Admins and Deep players; event-log buttons in `EventLog.svelte`). **Phase 1 truncates the future immediately (no undo).** *Phase 2 (remaining):* the greyed-out / undoable redo UX — roll back without discarding the future, grey later entries, and only commit the truncation when a new action is posted. The schema is already forward-compatible for this (add `Game.rollback_head_checkpoint_id`).
+- [x] **Rollback of game state.** Full-state JSON snapshots in a `Checkpoint` table, captured per action by the broadcast middleware, restored by `crud_rollback.rollback_to_checkpoint`; scene-confined; server-enforced for Admins and Deep players. Rolling back is non-destructive — it moves a head pointer (`Game.rollback_head_checkpoint_id`), so later log entries grey out and can be redone (roll forward); the abandoned future is only discarded when a new action is taken. UI is the per-event rollback/redo buttons + greyed entries + "rolled back" banner in `EventLog.svelte`.
 
 ## Words Words Words
 
@@ -10,3 +10,8 @@
 ## Security
 
 - [ ] **Defensive coding.** While we trust our players, this app is exposed on the open internet. I'd like to take basic precautions like making sure we're safe against SQL injection (SQLModel should handle this?), doing some basic sanitizing of free inputs from players (valid unicode only + generous character limit should be enough), and making sure that every game action API call is specific and constrained enough that it can't just do arbitrary database updates.
+
+## Cleanup
+
+- [ ] **Look for dead code.** While implementing the rollback feature, we discovered /scene/rollback, an unused and buggy single-card-play rollback route that was added but never hooked up to the UI. Look for other code that matches that pattern -- orphan functions and routes that never get called.
+- [ ] **Clean up ruff E701 warnings**

frontend/src/components/EventLog.svelte

@@ -39,6 +39,9 @@
     // Rollback buttons appear during a scene, for Admins and Deep players (server-enforced too).
     $: canRollback = state.game?.phase === 'scene'
         && (state.player?.is_admin || state.player?.role === 'deep');
+    // When set, the game is at a rolled-back state; events past this checkpoint are the
+    // greyed, still-undoable future. Rolling forward to one of them redoes the rollback.
+    $: head = state.game?.rollback_head_checkpoint_id ?? null;
 
     $: mergePolled(state.events);
 
@@ -46,8 +49,9 @@
     // otherwise events the player paged back to would vanish as the window slides.
     async function mergePolled(polled) {
         if (!polled) return;
-        // A rollback truncates the log server-side; the version bump tells us to drop
+        // Plain rollback/redo keeps every event (only `head` moves); but taking a new
-        // everything we'd accumulated (those future events no longer exist) and reseed.
+        // action from a rolled-back state truncates the abandoned future and bumps the
+        // version, which tells us to drop what we'd accumulated and reseed.
         const version = state.game?.rollback_timeline_version ?? 0;
         if (timelineVersion !== null && version !== timelineVersion) {
             events = [];
@@ -122,8 +126,10 @@
         return new Date(ts * 1000).toLocaleTimeString([], { hour: '2-digit', minute: '2-digit' });
     }
 
+    // Both rollback (to a past event) and redo (to a greyed future event) are the same
+    // call — it just moves the head. It's reversible until someone takes a new action,
+    // so no confirm; the greyed future makes the state obvious.
     async function rollback(event) {
-        if (!confirm(`Roll the game back to just after "${event.message}"? Everything after it will be undone.`)) return;
         error = '';
         try {
             await apiRequest(
@@ -147,6 +153,11 @@
         {#if error}
             <div class="log-error">{error}</div>
         {/if}
+        {#if head !== null}
+            <div class="log-rolledback">
+                ↩ Rolled back — greyed entries are undone. Click one to redo, or take an action to make it permanent.
+            </div>
+        {/if}
         <div class="log-content" bind:this={logEl} on:scroll={onScroll}>
             {#if events.length}
                 <div class="log-top">
@@ -159,16 +170,17 @@
                     {/if}
                 </div>
                 {#each events as event (event.id)}
-                    <div class="log-entry log-kind-{event.kind || 'info'}">
+                    {@const isFuture = head !== null && event.checkpoint_id > head}
+                    <div class="log-entry log-kind-{event.kind || 'info'}" class:greyed={isFuture}>
                         <span class="log-icon">{KIND_ICONS[event.kind] || KIND_ICONS.info}</span>
                         <span class="log-message">{event.message}</span>
                         <span class="log-time">{formatTime(event.timestamp)}</span>
                         {#if canRollback && event.checkpoint_id > 0}
                             <button
                                 class="rollback-btn"
-                                title="Roll the game back to just after this event"
+                                title={isFuture ? 'Redo forward to just after this event' : 'Roll the game back to just after this event'}
                                 on:click={() => rollback(event)}
-                            >↩</button>
+                            >{isFuture ? '↪' : '↩'}</button>
                         {/if}
                     </div>
                 {/each}
@@ -286,6 +298,20 @@
         color: var(--text);
         font-size: 0.8rem;
     }
+    .log-rolledback {
+        margin: 6px 8px 0;
+        padding: 6px 10px;
+        background: color-mix(in srgb, var(--accent) 12%, transparent);
+        border: 1px solid var(--accent);
+        border-radius: var(--radius-md);
+        color: var(--text);
+        font-size: 0.78rem;
+        line-height: 1.3;
+    }
+    .log-entry.greyed {
+        opacity: 0.45;
+        font-style: italic;
+    }
     .log-icon {
         font-size: 0.9rem;
     }

src/pirats/crud_rollback.py

@@ -15,6 +15,13 @@ Each event carries a `checkpoint_id` instead (see models.py):
   None = in-flight (created since the last capture; the next capture tags it)
   > 0  = a live rollback target within the current scene
   0    = sealed/historical (a past scene or non-scene event; never rollback-able)
+
+Rolling back is non-destructive: it restores a checkpoint's snapshot into the live
+tables and points `Game.rollback_head_checkpoint_id` at it, leaving the later
+checkpoints/events in place (the frontend greys them). Rolling forward to a later
+checkpoint "undoes" the rollback. The abandoned future is only discarded when a new
+action is taken from a rolled-back state (see maintain_rollback_history), which is
+what commits the new timeline.
 """
 
 import json
@@ -26,7 +33,7 @@ from .models import Game, Player, Obstacle, Challenge, Vote, GameEvent, Checkpoi
 
 # Game columns that are rollback *control* state, not gameplay — never snapshotted,
 # and preserved (not overwritten) when a snapshot is restored.
-SNAPSHOT_EXCLUDE = {"rollback_timeline_version"}
+SNAPSHOT_EXCLUDE = {"rollback_timeline_version", "rollback_head_checkpoint_id"}
 # Additionally never reassigned on restore (identity / secrets that don't change).
 _APPLY_SKIP = SNAPSHOT_EXCLUDE | {"id", "admin_key"}
 
@@ -117,23 +124,62 @@ def seal_and_purge(db: Session, game: Game) -> None:
 
     for cp in db.exec(select(Checkpoint).where(Checkpoint.game_id == game.id)).all():
         db.delete(cp)
+    game.rollback_head_checkpoint_id = None
+    db.add(game)
     db.commit()
 
 
+def _discard_future(db: Session, game: Game, after_checkpoint_id: int) -> None:
+    """Delete the checkpoints (and their events) created after a checkpoint — the
+    abandoned timeline when a new action is taken from a rolled-back state."""
+    for e in db.exec(
+        select(GameEvent).where(
+            GameEvent.game_id == game.id,
+            GameEvent.checkpoint_id > after_checkpoint_id,
+        )
+    ).all():
+        db.delete(e)
+    for c in db.exec(
+        select(Checkpoint).where(
+            Checkpoint.game_id == game.id,
+            Checkpoint.id > after_checkpoint_id,
+        )
+    ).all():
+        db.delete(c)
+
+
 def maintain_rollback_history(db: Session, game: Game) -> None:
     """Called after every successful mutation: snapshot the state while a scene is in
     progress, otherwise clear the (now-irrelevant) rollback history. This is the only
     decision the middleware makes; kept here so it can be unit-tested without HTTP."""
-    if game.phase == "scene":
+    if game.phase != "scene":
-        capture_checkpoint(db, game)
-    else:
         seal_and_purge(db, game)
+        return
+
+    # A new action taken from a rolled-back state commits the new timeline: discard the
+    # abandoned future first (its events are tagged with checkpoints past the head),
+    # then clear the head and bump the version so the frontend drops those events.
+    # (SQLite may reuse a discarded checkpoint's id for the next capture; that's safe
+    # since every event referencing it was just deleted.)
+    head = game.rollback_head_checkpoint_id
+    if head is not None:
+        _discard_future(db, game, head)
+        game.rollback_head_checkpoint_id = None
+        game.rollback_timeline_version += 1
+        db.add(game)
+        db.commit()
+
+    capture_checkpoint(db, game)
 
 
 def rollback_to_checkpoint(
     db: Session, game: Game, player: Player, checkpoint_id: int
 ) -> Tuple[bool, str]:
-    """Restore the game to the given checkpoint and discard everything after it.
+    """Restore the game to the given checkpoint and point the head at it.
+
+    Non-destructive: later checkpoints/events are kept (the frontend greys them) so the
+    rollback can be undone by rolling forward again; the same call serves both rollback
+    and redo. Rolling to the latest checkpoint clears the head (back to live play).
 
     Server-enforced: only during a scene, only by an Admin or a Deep player, and only
     to a checkpoint belonging to this game (which guarantees the current scene, since
@@ -149,25 +195,11 @@ def rollback_to_checkpoint(
 
     apply_game_state(db, game, cp.state_json)
 
-    # Truncate the future: events and checkpoints created after the target. Sealed
+    latest = db.exec(
-    # history (0) and the target's own events (== checkpoint_id) are kept.
+        select(Checkpoint).where(Checkpoint.game_id == game.id)
-    for e in db.exec(
+        .order_by(Checkpoint.id.desc())
-        select(GameEvent).where(
+    ).first()
-            GameEvent.game_id == game.id,
+    game.rollback_head_checkpoint_id = None if checkpoint_id == latest.id else checkpoint_id
-            GameEvent.checkpoint_id > checkpoint_id,
-        )
-    ).all():
-        db.delete(e)
-    for c in db.exec(
-        select(Checkpoint).where(
-            Checkpoint.game_id == game.id,
-            Checkpoint.id > checkpoint_id,
-        )
-    ).all():
-        db.delete(c)
-
-    # Bump so the frontend EventLog drops the events it had accumulated.
-    game.rollback_timeline_version += 1
     db.add(game)
     db.commit()
     return True, "Rolled back."

src/pirats/migrations/versions/e6294f6f8a81_add_rollback_head_pointer.py

@@ -0,0 +1,32 @@
+"""add rollback head pointer
+
+Revision ID: e6294f6f8a81
+Revises: 480e6d83109e
+Create Date: 2026-06-13 06:08:17.170588
+
+"""
+from alembic import op
+import sqlalchemy as sa
+import sqlmodel
+
+
+revision = 'e6294f6f8a81'
+down_revision = '480e6d83109e'
+branch_labels = None
+depends_on = None
+
+
+def upgrade() -> None:
+    # ### commands auto generated by Alembic - please adjust! ###
+    with op.batch_alter_table('game', schema=None) as batch_op:
+        batch_op.add_column(sa.Column('rollback_head_checkpoint_id', sa.Integer(), nullable=True))
+
+    # ### end Alembic commands ###
+
+
+def downgrade() -> None:
+    # ### commands auto generated by Alembic - please adjust! ###
+    with op.batch_alter_table('game', schema=None) as batch_op:
+        batch_op.drop_column('rollback_head_checkpoint_id')
+
+    # ### end Alembic commands ###

src/pirats/models.py

@@ -13,6 +13,7 @@ class Game(SQLModel, table=True):
     extra_obstacles: int = Field(default=0)  # Permanent increase to required obstacles (from Jokers drawn as Obstacles)
     deck_cards: str = Field(default="[]")  # JSON string of list of card codes, e.g. ["AS", "10D", "KH", ...]
     rollback_timeline_version: int = Field(default=0)  # Bumped whenever a rollback truncates events; lets the frontend reset its accumulated log. NEVER snapshotted (see crud_rollback.SNAPSHOT_EXCLUDE)
+    rollback_head_checkpoint_id: Optional[int] = Field(default=None)  # When set, the game is viewing a rolled-back state at this checkpoint; events after it are the greyed, still-undoable future. None = live at the latest. NEVER snapshotted.
     completed_crew_1: bool = Field(default=False)  # Steal a Ship
     completed_crew_2: bool = Field(default=False)  # Choose a Captain
     completed_crew_3: bool = Field(default=False)  # Commit Piracy

tests/test_game.py

@@ -2098,26 +2098,66 @@ def test_rollback_round_trip_restores_state(session):
     assert crud.get_game_deck(game) == saved_deck
     assert session.exec(select(Challenge).where(Challenge.game_id == game.id)).all() == []
 
-def test_rollback_truncates_future_and_bumps_version(session):
+def _scene_with_two_checkpoints(session, num_pirats=1):
-    game, deep, (p2,) = make_scene_game(session)
+    """A scene with sealed history and two action checkpoints (cp1 'First', cp2 'Second')."""
+    game, deep, pirats = make_scene_game(session, num_pirats=num_pirats)
     crud.seal_and_purge(session, game)
-
     crud.add_game_event(session, game.id, "First", kind="info")
     cp1 = crud.capture_checkpoint(session, game)
-    v0 = game.rollback_timeline_version
-
     crud.add_game_event(session, game.id, "Second", kind="info")
     cp2 = crud.capture_checkpoint(session, game)
-    assert {c.id for c in _checkpoints(session, game)} == {cp1.id, cp2.id}
+    return game, deep, pirats, cp1, cp2
+
+def test_rollback_keeps_future_and_sets_head(session):
+    game, deep, _, cp1, cp2 = _scene_with_two_checkpoints(session)
+    v0 = game.rollback_timeline_version
 
     ok, msg = crud.rollback_to_checkpoint(session, game, deep, cp1.id)
     assert ok, msg
     session.refresh(game)
 
-    # The future checkpoint and its event are gone; cp1 and its event survive.
+    # Non-destructive: the later checkpoint and its event are kept (the frontend greys
-    assert [c.id for c in _checkpoints(session, game)] == [cp1.id]
+    # them); head points at cp1 and the version is unchanged.
-    msgs = {e.message for e in _events(session, game)}
+    assert {c.id for c in _checkpoints(session, game)} == {cp1.id, cp2.id}
-    assert "First" in msgs and "Second" not in msgs
+    assert {e.message for e in _events(session, game)} >= {"First", "Second"}
+    assert game.rollback_head_checkpoint_id == cp1.id
+    assert game.rollback_timeline_version == v0
+
+def test_redo_after_rollback_clears_head(session):
+    game, deep, _, cp1, cp2 = _scene_with_two_checkpoints(session)
+
+    crud.rollback_to_checkpoint(session, game, deep, cp1.id)
+    session.refresh(game)
+    assert game.rollback_head_checkpoint_id == cp1.id
+
+    # Rolling forward to the latest checkpoint redoes the rollback and returns to live play.
+    ok, msg = crud.rollback_to_checkpoint(session, game, deep, cp2.id)
+    assert ok, msg
+    session.refresh(game)
+    assert game.rollback_head_checkpoint_id is None
+
+def test_action_after_rollback_truncates_and_bumps(session):
+    game, deep, _, cp1, cp2 = _scene_with_two_checkpoints(session)
+    v0 = game.rollback_timeline_version
+
+    crud.rollback_to_checkpoint(session, game, deep, cp1.id)
+
+    # A new action from the rolled-back state commits the new timeline: maintain_rollback_history
+    # discards the abandoned future, clears the head, bumps the version, and captures anew.
+    crud.add_game_event(session, game.id, "New branch", kind="info")
+    crud.maintain_rollback_history(session, game)
+    session.refresh(game)
+
+    events = _events(session, game)
+    msgs = {e.message for e in events}
+    assert "Second" not in msgs            # abandoned future discarded
+    assert {"First", "New branch"} <= msgs
+    # cp1 + the freshly captured checkpoint (note: SQLite may reuse cp2's freed id, which
+    # is safe — every event that referenced it was deleted in the same step).
+    assert len(_checkpoints(session, game)) == 2
+    new_event = next(e for e in events if e.message == "New branch")
+    assert new_event.checkpoint_id and new_event.checkpoint_id > 0
+    assert game.rollback_head_checkpoint_id is None
     assert game.rollback_timeline_version == v0 + 1
 
 def test_rollback_permissions(session):
@@ -2151,16 +2191,23 @@ def test_scene_end_seals_and_purges(session):
     crud.seal_and_purge(session, game)
     crud.add_game_event(session, game.id, "Mid-scene", kind="info")
     cp = crud.capture_checkpoint(session, game)
-    assert len(_checkpoints(session, game)) == 1
+    crud.add_game_event(session, game.id, "Later", kind="info")
+    crud.capture_checkpoint(session, game)  # a later checkpoint so the rollback sets a head
+    crud.rollback_to_checkpoint(session, game, deep, cp.id)  # leave with a head set
+    session.refresh(game)
+    assert game.rollback_head_checkpoint_id == cp.id
+    assert len(_checkpoints(session, game)) == 2
 
     # Leaving the scene runs the non-scene branch: history is purged and sealed.
     game.phase = "between_scenes"
     session.add(game)
     session.commit()
     crud.maintain_rollback_history(session, game)
+    session.refresh(game)
 
     assert _checkpoints(session, game) == []
     assert all(e.checkpoint_id == 0 for e in _events(session, game))
+    assert game.rollback_head_checkpoint_id is None
     # The now-stale checkpoint id is rejected (back in a scene, even).
     game.phase = "scene"
     session.add(game)
@@ -2211,13 +2258,30 @@ def test_rollback_middleware_capture_and_route_end_to_end():
         assert session.get(Obstacle, obs.id).current_value == 10
         assert len(_checkpoints(session, game)) == 2  # floor + the one the middleware took
 
-        # Roll back to the floor over HTTP.
+        # Roll back to the floor over HTTP: state reverts, but the later checkpoint is
+        # kept (greyed/undoable) and the head points at the floor.
         r = client.post(f"/api/game/{game.id}/player/{deep.id}/rollback",
                         data={"checkpoint_id": floor.id})
         assert r.status_code == 200
         session.expire_all()
         assert session.get(Obstacle, obs.id).current_value == 5
-        assert [c.id for c in _checkpoints(session, game)] == [floor.id]
+        assert len(_checkpoints(session, game)) == 2
+        session.refresh(game)
+        assert game.rollback_head_checkpoint_id == floor.id
+        v_before = game.rollback_timeline_version
+
+        # A new action from the rolled-back state (the floor restored the open challenge
+        # and the played card to p2's hand): the middleware truncates the abandoned future,
+        # clears the head, and bumps the version.
+        r = client.post(f"/api/game/{game.id}/player/{p2.id}/play-card",
+                        data={"obstacle_id": obs.id, "card_code": "10D"})
+        assert r.status_code == 200
+        session.expire_all()
+        session.refresh(game)
+        assert session.get(Obstacle, obs.id).current_value == 10
+        assert game.rollback_head_checkpoint_id is None
+        assert game.rollback_timeline_version == v_before + 1
+        assert len(_checkpoints(session, game)) == 2  # floor + the new branch's checkpoint
     finally:
         main_module.engine = saved_engine
         app.dependency_overrides.clear()