fix timers

backend/app/api/routes_timer.py

@@ -63,11 +63,18 @@ def resume(session: DbSession, settings: Settings) -> dict:
 @router.post("/complete")
 def complete(body: CompleteTimerRequest, session: DbSession, settings: Settings) -> dict:
     try:
-        finished, follow_on = pomodoro.complete(session, settings, auto_next=body.auto_next)
+        finished, follow_on = pomodoro.complete(
+            session,
+            settings,
+            auto_next=body.auto_next,
+            expect_id=body.session_id,
+        )
     except pomodoro.TimerError as exc:
         raise HTTPException(status_code=400, detail=str(exc)) from exc
 
-    celebration = _maybe_celebrate(session, settings) if finished.is_work else None
+    # `finished` is None when the named interval was no longer the running one:
+    # nothing happened, so there is nothing to celebrate and nothing to announce.
+    celebration = _maybe_celebrate(session, settings) if finished and finished.is_work else None
     state = pomodoro.state(session, settings)
     state["finished"] = pomodoro.session_dict(finished)
     state["auto_started"] = pomodoro.session_dict(follow_on)

backend/app/api/schemas.py

@@ -29,6 +29,16 @@ class AttachTaskRequest(BaseModel):
 
 class CompleteTimerRequest(BaseModel):
     auto_next: bool = True
+    session_id: int | None = None
+    """Which interval the client believes it is finishing.
+
+    Every client ticks its own local countdown and calls `complete` when it
+    reaches zero, so with two of them open (the phone and the desktop, say)
+    both fire within the same second. Unconditionally, the second call lands
+    on whatever `auto_next` has just started and ends *that* — which is why a
+    break could vanish the instant it began. Sent along, a stale call is a
+    no-op instead. Omitted, the old unconditional behaviour stands.
+    """
 
 
 class TaskCreateRequest(BaseModel):

backend/app/services/pomodoro.py

@@ -176,10 +176,28 @@ def resume(session: Session) -> PomodoroSession:
 
 
 def complete(
-    session: Session, settings: AppSettings, *, auto_next: bool = True
-) -> tuple[PomodoroSession, PomodoroSession | None]:
-    """Finish the current interval and, if configured, roll into the next one."""
-    row = _require_active(session)
+    session: Session,
+    settings: AppSettings,
+    *,
+    auto_next: bool = True,
+    expect_id: int | None = None,
+) -> tuple[PomodoroSession | None, PomodoroSession | None]:
+    """Finish the current interval and, if configured, roll into the next one.
+
+    `expect_id` is the interval the caller believes it is finishing. Every
+    client runs its own local countdown and calls this when it hits zero, so
+    two clients open at once both call within the same second — and the second
+    call used to land on the interval `auto_next` had just created and end it
+    immediately. That is a break disappearing the moment it starts, and it is
+    not something a client can fix on its own: only the server knows which
+    interval is current. So a call that names an interval which is no longer
+    running does nothing at all, and says so by returning no `finished`.
+    """
+    row = current_session(session)
+    if expect_id is not None and (row is None or row.id != expect_id):
+        return None, None
+    if row is None:
+        raise TimerError("No interval is running.")
     finished = _finish(session, settings, row, status="completed")
 
     follow_on: PomodoroSession | None = None

backend/tests/test_api.py

@@ -29,6 +29,28 @@ def test_a_whole_pomodoro_through_the_api(client):
     assert "Daily report" in report["telegram_preview"]
 
 
+def test_a_stale_complete_leaves_the_auto_started_break_alone(client):
+    client.put(
+        "/api/settings",
+        json={"pomodoro": {"auto_start_breaks": True, "auto_start_work": True}},
+    )
+    state = client.post("/api/timer/start", json={"kind": "work"}).json()
+    work_id = state["current"]["id"]
+
+    done = client.post(
+        "/api/timer/complete", json={"auto_next": True, "session_id": work_id}
+    ).json()
+    assert done["current"]["kind"] in ("short_break", "long_break")
+    break_id = done["current"]["id"]
+
+    # The other client's countdown, one beat behind.
+    again = client.post(
+        "/api/timer/complete", json={"auto_next": True, "session_id": work_id}
+    ).json()
+    assert again["finished"] is None
+    assert again["current"]["id"] == break_id
+
+
 def test_timer_errors_are_400_not_500(client):
     assert client.post("/api/timer/pause").status_code == 400
     assert client.post("/api/timer/complete", json={}).status_code == 400

backend/tests/test_pomodoro.py

@@ -69,6 +69,40 @@ def test_auto_start_breaks_rolls_straight_on(session, settings):
     assert follow_on is not None and follow_on.kind == "short_break"
 
 
+def test_a_complete_naming_a_finished_interval_does_nothing(session, settings):
+    """Two clients reaching zero at once must not cost you the break.
+
+    Every client ticks its own countdown and calls `complete` when it runs out.
+    The first call finishes the work interval and auto-starts the break; the
+    second — sent a moment earlier, from a client that has not polled since —
+    used to finish *that* break, so a break ended the instant it began.
+    """
+    settings.pomodoro.auto_start_breaks = True
+    settings.pomodoro.auto_start_work = True
+    settings_service.save_settings(session, settings)
+
+    work = pomodoro.start(session, settings, kind="work")
+    finished, follow_on = pomodoro.complete(session, settings, expect_id=work.id)
+    assert finished is not None and follow_on is not None
+    assert follow_on.kind == "short_break"
+
+    # The second client's call, naming the work interval it was still showing.
+    stale_finished, stale_follow_on = pomodoro.complete(session, settings, expect_id=work.id)
+    assert stale_finished is None and stale_follow_on is None
+
+    still_running = pomodoro.current_session(session)
+    assert still_running is not None
+    assert still_running.id == follow_on.id and still_running.status == "running"
+
+
+def test_complete_without_a_session_id_stays_unconditional(session, settings):
+    settings.pomodoro.auto_start_breaks = False
+    settings_service.save_settings(session, settings)
+    pomodoro.start(session, settings, kind="work")
+    finished, _ = pomodoro.complete(session, settings)
+    assert finished is not None and finished.status == "completed"
+
+
 def test_strict_mode_demands_a_reason(session, settings):
     settings.pomodoro.strict_mode = True
     settings_service.save_settings(session, settings)