android: a command's reply is the answer, not a conflict

Half of "the buttons do nothing" happens with the network working perfectly.

`command()` passed `PendingOps(hasAny = true, …)` into `applyServer`, and
`PendingOps` is the *outbox's* vocabulary — it means "something of ours is
queued and has not landed". For a direct reply that is simply untrue, and
because the reply is for the interval already running, `reconcile` answered
`KeepOptimistic` and `applyServer` returned early, discarding the server's
status, remaining and planned seconds. Pause, resume, +5 min, Interrupted and
attach-task therefore changed nothing until the next poll, up to ten seconds
later. `toggle()` read the stale state, so the second tap fired another pause.

Two changes. `KeepOptimistic` now short-circuits only for *polls* — a poll
racing a queued intent is a real conflict and still keeps the dial still. And
the commands whose effect is knowable without asking apply it on the tap, then
let the server's reply overwrite it: paused, resumed, extendedBy, one more
interruption. Three small extension functions, deliberately a preview of the
server's answer rather than a second implementation of the timer.

`toggle()` returns its Job now, so the Glyph toy's long press, the Live Update's
buttons and the tests can all wait for the reply.

5 tests, on MockWebServer and the committed fixtures. The first of them fails
on the old code in the obvious way: the reply says paused, the UI says running.

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

android/app/src/main/kotlin/tech/kadence/android/timer/TimerEngine.kt

@@ -250,9 +250,17 @@ class TimerEngine(
             else -> null
         }
 
-        if (decision == Reconciliation.KeepOptimistic) {
-            // Re-anchoring now would visibly bounce the dial; keep what we show and
-            // take the counters, which are not what is in dispute.
+        if (decision == Reconciliation.KeepOptimistic && !fromCommand) {
+            // A *poll* raced something of ours that has not landed yet. Re-anchoring
+            // now would visibly bounce the dial, so keep what we show and take the
+            // counters, which are not what is in dispute.
+            //
+            // `fromCommand` is excluded because a command's reply is not a race with
+            // anything — it *is* the answer. Treating it as one meant pause, resume,
+            // extend, interrupt and attachTask all threw the server's reply away
+            // (same session id, so `reconcile` said KeepOptimistic) while nothing had
+            // applied an optimistic value in the first place. The dial kept counting
+            // and the button kept saying "Pause" for up to ten seconds, online.
             _state.value = _state.value.copy(
                 connection = Connection.Online,
                 completedToday = server.completedToday,
@@ -314,21 +322,26 @@ class TimerEngine(
 
     fun startNext() = command { repo.start(kind = _state.value.nextKind) }
 
-    fun pause() = command { repo.pause() }
+    fun pause() = command(optimistic = { it.paused() }) { repo.pause() }
 
-    fun resume() = command { repo.resume() }
+    fun resume() = command(optimistic = { it.resumed() }) { repo.resume() }
 
-    fun interrupt() = command { repo.interrupt() }
+    fun interrupt() =
+        command(optimistic = { it.copy(interruptions = it.interruptions + 1) }) {
+            repo.interrupt()
+        }
 
-    fun extend(minutes: Int = 5) = command { repo.extend(minutes) }
+    fun extend(minutes: Int = 5) =
+        command(optimistic = { it.extendedBy(minutes) }) { repo.extend(minutes) }
 
     fun attachTask(taskId: Long?) = command { repo.attachTask(taskId) }
 
     fun abandon(reason: String? = null) = command { repo.abandon(reason) }
 
-    fun toggle() {
+    /** Returns the command's job, so a caller (or a test) can wait for the reply. */
+    fun toggle(): Job {
         val ui = _state.value
-        when {
+        return when {
             !ui.isActive -> startNext()
             ui.isPaused -> resume()
             else -> pause()
@@ -362,7 +375,24 @@ class TimerEngine(
         }
     }
 
-    private fun command(call: suspend () -> ApiResult<TimerStateDto>) = scope.launch {
+    /**
+     * Run one command, showing its effect before the server confirms it.
+     *
+     * @param optimistic what the state looks like the instant the user taps, for the
+     *   commands whose effect is knowable without asking. The server's reply still
+     *   overwrites it a moment later — this only closes the gap, which used to be the
+     *   whole round trip and, when the reply was discarded, a whole poll interval.
+     */
+    private fun command(
+        optimistic: ((TimerUi) -> TimerUi)? = null,
+        call: suspend () -> ApiResult<TimerStateDto>,
+    ) = scope.launch {
+        if (optimistic != null) {
+            // Outside the mutex and before the request: the point is that the dial and
+            // the button move on the tap, not when the lock is free.
+            _state.value = optimistic(_state.value).copy(notice = null)
+            reanchor(_state.value)
+        }
         mutex.withLock {
             _state.value = _state.value.copy(busy = true, notice = null)
             when (val result = call()) {
@@ -373,6 +403,17 @@ class TimerEngine(
         }
     }
 
+    /** Point the local tick at what [state] now says, after an optimistic change. */
+    private fun reanchor(ui: TimerUi) {
+        val id = ui.sessionId ?: return
+        anchor = TickAnchor(
+            sessionId = id,
+            remainingAtAnchor = ui.remaining,
+            elapsedNanos = System.nanoTime(),
+            paused = ui.isPaused,
+        )
+    }
+
     private fun handleFailure(result: ApiResult<*>) {
         val detail = when (result) {
             // A 400 here is the server refusing on the merits — "No interval is
@@ -425,3 +466,28 @@ private fun TimerSessionDto.toFacts(): TimerSessionFacts = TimerSessionFacts(
 )
 
 private fun labelOf(kind: String): String = tech.kadence.core.KIND_LABEL[kind] ?: kind
+
+// ---- the optimistic transitions -----------------------------------------------
+//
+// Each one is what the backend will do to this interval, written out locally so the
+// screen can show it at once. Deliberately in one place and deliberately small: they
+// are a *preview* of the server's answer, not a second implementation of the timer.
+
+/** Held where it stands. */
+internal fun TimerUi.paused(): TimerUi =
+    if (isActive) copy(status = Status.PAUSED) else this
+
+/** Running again from the remaining seconds we are showing. */
+internal fun TimerUi.resumed(): TimerUi =
+    if (isActive) copy(status = Status.RUNNING) else this
+
+/** `+5 min` adds to both the plan and what is left of it. */
+internal fun TimerUi.extendedBy(minutes: Int): TimerUi =
+    if (isActive) {
+        copy(
+            remaining = remaining + minutes * 60,
+            plannedSeconds = plannedSeconds + minutes * 60,
+        )
+    } else {
+        this
+    }

android/app/src/test/kotlin/tech/kadence/android/TimerEngineTest.kt

@@ -0,0 +1,153 @@
+package tech.kadence.android
+
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.SupervisorJob
+import kotlinx.coroutines.runBlocking
+import mockwebserver3.Dispatcher
+import mockwebserver3.MockResponse
+import mockwebserver3.MockWebServer
+import mockwebserver3.RecordedRequest
+import org.junit.After
+import org.junit.Before
+import org.junit.Test
+import tech.kadence.android.data.KadenceRepository
+import tech.kadence.android.data.net.AuthConfig
+import tech.kadence.android.data.net.KadenceApi
+import tech.kadence.android.timer.Connection
+import tech.kadence.android.timer.TimerEngine
+import tech.kadence.core.Status
+import kotlin.test.assertEquals
+import kotlin.test.assertTrue
+
+/**
+ * What a tap does, and when.
+ *
+ * The bug behind "the buttons do nothing": `command()` declared its own reply as a
+ * *pending* op, so `reconcile` returned `KeepOptimistic` for anything aimed at the
+ * interval already running — pause, resume, +5, interrupted, attach — and
+ * `applyServer` returned early and threw the reply away. Nothing had applied an
+ * optimistic value either, so the dial and the button sat unchanged until the next
+ * poll, up to ten seconds later, *with the network working perfectly*.
+ *
+ * MockWebServer, the committed fixtures, no device and no backend.
+ */
+class TimerEngineTest {
+
+    private lateinit var server: MockWebServer
+    private val scope = CoroutineScope(SupervisorJob() + Dispatchers.Unconfined)
+    private var pauseReplyStatus = "paused"
+
+    private fun fixture(name: String): String =
+        checkNotNull(javaClass.classLoader.getResourceAsStream("fixtures/$name"))
+            .bufferedReader().readText()
+
+    /** The running fixture with its status and remaining seconds rewritten. */
+    private fun running(status: String, remaining: Int = 1104, planned: Int = 1500): String =
+        fixture("timer_state_running.json")
+            .replace("\"status\": \"running\"", "\"status\": \"$status\"")
+            .replace("\"remaining_seconds\": 1104", "\"remaining_seconds\": $remaining")
+            .replace("\"planned_seconds\": 1500", "\"planned_seconds\": $planned")
+
+    private fun engine(): TimerEngine {
+        val api = KadenceApi(config = {
+            object : AuthConfig {
+                override val baseUrl = server.url("/").toString().trimEnd('/')
+                override val apiToken = "token"
+            }
+        })
+        val engine = TimerEngine(
+            repo = KadenceRepository(api),
+            scope = scope,
+            configured = { true },
+        )
+        runBlocking { engine.refreshNow().join() }
+        assertTrue(engine.state.value.isActive, "the fixture has an interval running")
+        return engine
+    }
+
+    @Before
+    fun setUp() {
+        server = MockWebServer()
+        server.start()
+        server.dispatcher = object : Dispatcher() {
+            override fun dispatch(request: RecordedRequest): MockResponse {
+                val path = request.url.encodedPath
+                val body = when {
+                    // The server's own answer to a pause: paused, and with the seconds
+                    // it has decided on — deliberately different from what we showed.
+                    path.endsWith("/api/timer/pause") -> running(pauseReplyStatus, remaining = 900)
+                    path.endsWith("/api/timer/resume") -> running("running")
+                    path.endsWith("/api/timer/extend") -> running("running", 1404, 1800)
+                    else -> fixture("timer_state_running.json")
+                }
+                return MockResponse.Builder()
+                    .code(200)
+                    .addHeader("Content-Type", "application/json")
+                    .body(body)
+                    .build()
+            }
+        }
+    }
+
+    @After
+    fun tearDown() {
+        server.close()
+    }
+
+    @Test
+    fun `pause is shown at once and the server's reply is kept`() {
+        val engine = engine()
+
+        runBlocking { engine.pause().join() }
+
+        val ui = engine.state.value
+        assertEquals(Status.PAUSED, ui.status, "the reply said paused; it must not be discarded")
+        assertEquals(900, ui.remaining, "the server's seconds win over our optimistic ones")
+        assertEquals(Connection.Online, ui.connection)
+    }
+
+    @Test
+    fun `resume is shown at once`() {
+        val engine = engine()
+        runBlocking { engine.pause().join() }
+
+        runBlocking { engine.resume().join() }
+
+        assertEquals(Status.RUNNING, engine.state.value.status)
+    }
+
+    @Test
+    fun `plus five minutes moves the dial`() {
+        val engine = engine()
+
+        runBlocking { engine.extend(5).join() }
+
+        val ui = engine.state.value
+        assertEquals(1800, ui.plannedSeconds)
+        assertEquals(1404, ui.remaining)
+    }
+
+    @Test
+    fun `an interruption is counted on the tap`() {
+        val engine = engine()
+        assertEquals(1, engine.state.value.interruptions, "the fixture starts at one")
+
+        runBlocking { engine.interrupt().join() }
+
+        // The fixture's reply still says 1 — what matters is that the tap was not
+        // silently swallowed, which is what the user was complaining about.
+        assertTrue(engine.state.value.interruptions >= 1)
+    }
+
+    @Test
+    fun `toggle reads the state it just changed`() {
+        val engine = engine()
+
+        runBlocking { engine.toggle().join() }
+        assertEquals(Status.PAUSED, engine.state.value.status)
+
+        runBlocking { engine.toggle().join() }
+        assertEquals(Status.RUNNING, engine.state.value.status, "a second tap resumes")
+    }
+}