Fix Workbench Play stall after the first live step

docs/agent/feature-map.md

@@ -47,6 +47,6 @@ The canonical worker bootstrap environment is `PKB_CONTROL_BRIDGE_SESSION_DIR`,
 
 `WorkbenchLiveSession` binds operations to the controller-owned paused scenario and verifies worker PID, bridge runtime id, and scenario id stability. Step Override source is compiled and loaded worker-side. Normal live calls do not invoke Maven/Gradle, resynchronize, or restart the worker.
 
-The lightweight non-Spring stdio MCP adapter and player-style Swing/WebView UI both delegate through `WorkbenchServices` / `WorkbenchController`. `LiveScenarioPlayer` is a Workbench-side headless presentation model for the editable session buffer, stable line identities, selection, click-to-seek playhead, and player state only; it does not parse or execute Pickleball steps, model ParsingMap/NodeMap semantics, or claim runtime rewind. The controller also owns the watched-agent control lease (`HUMAN` / `AGENT`, banner `currentAction`, gated Save permission). UI mode writes a localhost attach endpoint to `.pickleball/workbench/attach.json` so an agent joins the same JVM/worker instead of starting a second Workbench. The left-rail picker reads project-owned feature files. The live editor is a Gherkin block WebView over that same player model. Global Play starts from the first executable step; Step Editor **Step** is isolated `executeStep`; **From Here** runs from the selected step; wait-at-end stays in play so Enter can append-and-continue. Mapping property types go through `mappingPut` / `mappingRestore`. Terminal tails existing worker log files. Diagnostic explorer reads retained `reports/diagnostic-runs` artifacts in the repository evidence order. The default buffer is a Workbench-owned browser demo against `URL.home`; **Save** is confirmation-gated and copies only a picker-loaded scenario back to its originating `.feature` file. OpenJFX is Workbench-only. MCP mode reserves stdout for newline-delimited MCP JSON-RPC and redirects ordinary output to stderr. The UI remains execution-oriented and does not recreate a project IDE, generic build/process UI, source navigator, or collaboration system.
+The lightweight non-Spring stdio MCP adapter and player-style Swing/WebView UI both delegate through `WorkbenchServices` / `WorkbenchController`. `LiveScenarioPlayer` is a Workbench-side headless presentation model for the editable session buffer, stable line identities, selection, click-to-seek playhead, and player state only; it does not parse or execute Pickleball steps, model ParsingMap/NodeMap semantics, or claim runtime rewind. While `RUNNING`, `WorkbenchController.executeStep` owns playhead follow once (`LivePlaybackCoordinator.followExecutedStep`); the Swing Play loop continues without remaking that mark, and leftover marks of an already-consumed step are no-ops. The controller also owns the watched-agent control lease (`HUMAN` / `AGENT`, banner `currentAction`, gated Save permission). UI mode writes a localhost attach endpoint to `.pickleball/workbench/attach.json` so an agent joins the same JVM/worker instead of starting a second Workbench. The left-rail picker reads project-owned feature files. The live editor is a Gherkin block WebView over that same player model. Global Play starts from the first executable step; Step Editor **Step** is isolated `executeStep`; **From Here** runs from the selected step; wait-at-end stays in play so Enter can append-and-continue. Mapping property types go through `mappingPut` / `mappingRestore`. Terminal tails existing worker log files. Diagnostic explorer reads retained `reports/diagnostic-runs` artifacts in the repository evidence order. The default buffer is a Workbench-owned browser demo against `URL.home`; **Save** is confirmation-gated and copies only a picker-loaded scenario back to its originating `.feature` file. OpenJFX is Workbench-only. MCP mode reserves stdout for newline-delimited MCP JSON-RPC and redirects ordinary output to stderr. The UI remains execution-oriented and does not recreate a project IDE, generic build/process UI, source navigator, or collaboration system.
 
 For this boundary, scenario validation is limited to the affected `@control-bridge` and/or `@step-override-bridge` tags with `pkb_parallel=80` where practical. Do not run `@all` for Workbench isolation changes.

docs/pickleball-workbench-player.md

@@ -59,6 +59,7 @@ Scenario: Open the local test site
   - **Step** executes only the Step Editor text against the current paused live context and leaves automatic scenario playback paused.
   - **From Here** creates a fresh interactive scenario context and treats the selected/playhead executable step as the first step of that run, then continues through the remaining buffer.
 - Fresh scenario playback restarts the consumer worker so browser, Mapping, service, and other side effects from a previous run do not leak into a new **Play** or **From Here** run.
+- Automatic **Play** / **From Here** send each executable live-buffer line through `executeStep`. While the player is `RUNNING`, that controller call is the single playhead owner: success advances to the next executable line, failure pauses on the failed line. The Swing Play loop then refreshes and schedules the next line without remaking the same mark. An attached agent `execute_step` uses the same follow so the spectator playhead stays aligned. Isolated **Step** pauses first, so it does not move the playhead. Marking an already-consumed step is a no-op, so a leftover UI callback cannot abort playback after a successful worker step.
 - Reaching the end while playing changes the player to **Waiting for step** rather than stopping. Typing a new step and pressing **Enter** appends it to the end of the live scenario and executes it as part of the same live run.
 - **Ctrl+Enter** updates the selected line in place. The whole-scenario editor also accepts ordinary typing at any line.
 - The editor highlights the current playhead line. Successful lines do not retain checkmarks or become locked.
@@ -126,7 +127,7 @@ The included consumer `@control-bridge` scenario verifies:
 - the current ParsingMap catalog contains at least one NodeMap;
 - a catalog reference resolves back to a live NodeMap.
 
-Workbench player/editor unit tests cover picker selection/search, block buffer ↔ player model, click-to-seek, global Play from start, the two Step Editor play actions, wait-at-end / Enter-to-append-and-run, in-place edit of previously executed text, typed Mapping edits through `WorkbenchServices`, the non-empty browser demo seed, control-lease lock/Take control, permission grant/deny, and Save not writing without approval.
+Workbench player/editor unit tests cover picker selection/search, block buffer ↔ player model, click-to-seek, global Play from start, the two Step Editor play actions, wait-at-end / Enter-to-append-and-run, in-place edit of previously executed text, leftover Play-loop playhead marks after `executeStep`, typed Mapping edits through `WorkbenchServices`, the non-empty browser demo seed, control-lease lock/Take control, permission grant/deny, and Save not writing without approval.
 
 Workbench changes should continue to use the repository's focused validation policy:
 

docs/pickleball-workbench.md

@@ -154,7 +154,7 @@ The Live Scenario Editor is a session-scoped Gherkin document presented as snap-
 The playhead behaves like an audio-player needle:
 
 - clicking a scenario line instantly seeks the playhead to that line;
-- while a run is active, the playhead follows the current/next executable step after each success;
+- while a run is active, `executeStep` advances the playhead once on success (or pauses it on failure); the UI Play loop continues from that new next step without remaking the same mark;
 - **Pause** and **Stop** do not claim to rewind browser, Mapping, service, or other worker side effects.
 
 Global **Play** always starts a fresh interactive scenario context and runs from the first executable step, even if the playhead is elsewhere. Fresh **Play** / **From Here** runs restart the consumer worker so prior side effects do not masquerade as the start of a scenario.

pickleball-workbench/src/main/java/tools/dscode/workbench/WorkbenchController.java

@@ -365,18 +365,10 @@ public void close() {
     }
 
     private void maybeAdvancePlayhead(String text, boolean successful) {
-        LiveScenarioPlayer.Line next = player.nextStep().orElse(null);
-        if (next != null && text != null && next.text().equals(text)
-                && player.state() == LiveScenarioPlayer.State.RUNNING) {
-            try {
-                if (successful) {
-                    player.markCurrentStepExecuted(next.id());
-                } else {
-                    player.markCurrentStepFailed(next.id());
-                }
-            } catch (RuntimeException ignored) {
-                // Playhead follow is best-effort presentation; worker execution already finished.
-            }
+        try {
+            playback.followExecutedStep(text, successful);
+        } catch (RuntimeException ignored) {
+            // Playhead follow is best-effort presentation; worker execution already finished.
         }
         notifyPlayer();
     }

pickleball-workbench/src/main/java/tools/dscode/workbench/player/LivePlaybackCoordinator.java

@@ -106,4 +106,24 @@ public boolean waitingForStep() {
     public boolean running() {
         return player.state() == LiveScenarioPlayer.State.RUNNING;
     }
+
+    /**
+     * Single owner for playhead follow after a worker {@code executeStep}.
+     * Advances only while {@link LiveScenarioPlayer.State#RUNNING} and only when
+     * the executed text is the current next step. Isolated Step Only pauses first,
+     * so it does not move the playhead. Attached-agent {@code execute_step} uses
+     * this same follow while a UI Play run is in progress.
+     */
+    public void followExecutedStep(String text, boolean successful) {
+        LiveScenarioPlayer.Line next = player.nextStep().orElse(null);
+        if (next == null || text == null || !next.text().equals(text)
+                || player.state() != LiveScenarioPlayer.State.RUNNING) {
+            return;
+        }
+        if (successful) {
+            player.markCurrentStepExecuted(next.id());
+        } else {
+            player.markCurrentStepFailed(next.id());
+        }
+    }
 }

pickleball-workbench/src/main/java/tools/dscode/workbench/player/LiveScenarioPlayer.java

@@ -283,9 +283,16 @@ public void pauseForIsolatedExecution() {
         state = State.PAUSED;
     }
 
-    /** Advances a successful run to the next executable line and stays in play at end. */
+    /**
+     * Advances a successful run to the next executable line and stays in play at end.
+     * Already-consumed or stale ids are ignored so a leftover Play-loop callback
+     * cannot abort automatic playback.
+     */
     public void markCurrentStepExecuted(long stepId) {
-        int index = requireCurrentStep(stepId);
+        int index = currentExecutableIndex(stepId);
+        if (index < 0) {
+            return;
+        }
         lastExecutedId = stepId;
         executionIndex = findNextExecutableIndex(index + 1);
         if (executionIndex < lines.size()) {
@@ -295,9 +302,16 @@ public void markCurrentStepExecuted(long stepId) {
         }
     }
 
-    /** Leaves a failed run paused on its failed line. */
+    /**
+     * Leaves a failed run paused on its failed line. Already-consumed or stale
+     * ids are ignored.
+     */
     public void markCurrentStepFailed(long stepId) {
-        executionIndex = requireCurrentStep(stepId);
+        int index = currentExecutableIndex(stepId);
+        if (index < 0) {
+            return;
+        }
+        executionIndex = index;
         playheadId = stepId;
         selectedId = stepId;
         state = State.PAUSED;
@@ -357,10 +371,10 @@ private int findNextExecutableIndex(int from) {
         return lines.size();
     }
 
-    private int requireCurrentStep(long id) {
+    private int currentExecutableIndex(long id) {
         if (executionIndex >= lines.size() || !lines.get(executionIndex).executable()
                 || lines.get(executionIndex).id() != id) {
-            throw new IllegalStateException("Step " + id + " is not the current execution step.");
+            return -1;
         }
         return executionIndex;
     }

pickleball-workbench/src/main/java/tools/dscode/workbench/ui/WorkbenchFrame.java

@@ -948,10 +948,10 @@ private void schedulePlaybackStep() {
                     executingStepId = null;
                     appendTerminal(step.text(), result.output(), result.events());
 
-                    if (result.successful()) {
-                        player.markCurrentStepExecuted(step.id());
-                    } else {
-                        player.markCurrentStepFailed(step.id());
+                    // executeStep already advanced or paused the playhead while RUNNING.
+                    // Do not remake that mark here; a leftover mark of the captured id
+                    // used to abort automatic playback after the first successful step.
+                    if (!result.successful()) {
                         player.clickLine(step.id());
                         showLine(step.id());
                     }

pickleball-workbench/src/test/java/tools/dscode/workbench/player/LivePlaybackCoordinatorTest.java

@@ -4,7 +4,9 @@
 
 import java.util.List;
 
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 class LivePlaybackCoordinatorTest {
@@ -64,4 +66,78 @@ void loadingAScenarioReplacesTheLiveBufferWithoutWritingFiles() {
         assertTrue(coordinator.player().documentText().contains("navigate to: URL.home"));
         assertTrue(coordinator.origin().savable() == false);
     }
+
+    @Test
+    void executeStepOwnsThePlayheadSoALeftoverUiMarkCannotStallPlay() {
+        LivePlaybackCoordinator coordinator = new LivePlaybackCoordinator(LiveScenarioPlayer.interactiveBuffer());
+        LiveScenarioPlayer player = coordinator.player();
+        coordinator.playFromStart();
+
+        LiveScenarioPlayer.Line first = player.nextStep().orElseThrow();
+        assertTrue(first.text().contains("navigate to: URL.home"));
+        assertEquals(LiveScenarioPlayer.State.RUNNING, player.state());
+
+        coordinator.followExecutedStep(first.text(), true);
+        assertDoesNotThrow(() -> simulateFrameSuccessCallback(player, first));
+
+        LiveScenarioPlayer.Line second = player.nextStep().orElseThrow();
+        assertNotEquals(first.id(), second.id());
+        assertEquals(LiveScenarioPlayer.State.RUNNING, player.state());
+
+        int walked = 1;
+        while (player.state() == LiveScenarioPlayer.State.RUNNING && player.nextStep().isPresent()) {
+            LiveScenarioPlayer.Line step = player.nextStep().orElseThrow();
+            coordinator.followExecutedStep(step.text(), true);
+            assertDoesNotThrow(() -> simulateFrameSuccessCallback(player, step));
+            walked++;
+        }
+
+        assertTrue(walked >= 3, "demo buffer should walk more than the first live step, walked=" + walked);
+        assertEquals(LiveScenarioPlayer.State.WAITING_FOR_STEP, player.state());
+        assertTrue(player.nextStep().isEmpty());
+    }
+
+    @Test
+    void agentExecuteStepAdvancesThePlayheadOnceWhileRunning() {
+        LivePlaybackCoordinator coordinator = new LivePlaybackCoordinator(new LiveScenarioPlayer(List.of(
+                "Given first",
+                "And second",
+                "Then third"
+        )));
+        LiveScenarioPlayer player = coordinator.player();
+        coordinator.playFromStart();
+        LiveScenarioPlayer.Line first = player.nextStep().orElseThrow();
+
+        coordinator.followExecutedStep(first.text(), true);
+        assertEquals("And second", player.nextStep().orElseThrow().text());
+        assertEquals(LiveScenarioPlayer.State.RUNNING, player.state());
+
+        coordinator.followExecutedStep(first.text(), true);
+        assertEquals("And second", player.nextStep().orElseThrow().text());
+    }
+
+    @Test
+    void isolatedStepOnlyDoesNotAdvanceThePlayhead() {
+        LivePlaybackCoordinator coordinator = new LivePlaybackCoordinator(new LiveScenarioPlayer(List.of(
+                "Given first",
+                "And second"
+        )));
+        LiveScenarioPlayer player = coordinator.player();
+        coordinator.playFromStart();
+        LiveScenarioPlayer.Line first = player.nextStep().orElseThrow();
+
+        coordinator.stepOnly();
+        coordinator.followExecutedStep(first.text(), true);
+
+        assertEquals(LiveScenarioPlayer.State.PAUSED, player.state());
+        assertEquals(first.id(), player.nextStep().orElseThrow().id());
+    }
+
+    /**
+     * Former WorkbenchFrame success callback: remake the captured step mark after
+     * executeStep already followed the playhead, then continue while RUNNING.
+     */
+    private static void simulateFrameSuccessCallback(LiveScenarioPlayer player, LiveScenarioPlayer.Line captured) {
+        player.markCurrentStepExecuted(captured.id());
+    }
 }

pickleball-workbench/src/test/java/tools/dscode/workbench/player/LiveScenarioPlayerTest.java

@@ -5,6 +5,7 @@
 import java.util.ArrayList;
 import java.util.List;
 
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
@@ -226,4 +227,32 @@ void insertingIntoCompletedMiddleDoesNotReplayLaterSteps() {
         assertTrue(player.nextStep().isEmpty());
         assertNotEquals("And inserted in middle", player.lines().getFirst().text());
     }
+
+    @Test
+    void leftoverPlayheadMarksAreIdempotentAndDoNotThrow() {
+        LiveScenarioPlayer player = new LiveScenarioPlayer(List.of(
+                "Given first",
+                "And second",
+                "Then third"
+        ));
+        player.startFromBeginning();
+        long first = player.nextStep().orElseThrow().id();
+        long second = player.lines().get(1).id();
+
+        assertDoesNotThrow(() -> player.markCurrentStepExecuted(first));
+        assertDoesNotThrow(() -> player.markCurrentStepExecuted(first));
+        assertDoesNotThrow(() -> player.markCurrentStepExecuted(second + 99));
+
+        assertEquals(second, player.nextStep().orElseThrow().id());
+        assertEquals(LiveScenarioPlayer.State.RUNNING, player.state());
+
+        assertDoesNotThrow(() -> player.markCurrentStepFailed(first));
+        assertEquals(LiveScenarioPlayer.State.RUNNING, player.state());
+        assertEquals(second, player.nextStep().orElseThrow().id());
+
+        assertDoesNotThrow(() -> player.markCurrentStepFailed(second));
+        assertDoesNotThrow(() -> player.markCurrentStepFailed(second));
+        assertEquals(LiveScenarioPlayer.State.PAUSED, player.state());
+        assertEquals(second, player.nextStep().orElseThrow().id());
+    }
 }