fix(playback): correct nine faults in progress sync

AGENTS.md

@@ -115,8 +115,8 @@ The server is written from the MPRIS 2.2 specification. The Go MPRIS server libr
 | `client.go` | HTTP client with 10s timeout, 50MB response limit, auth header, request logging |
 | `types.go` | All ABS data types. Custom UnmarshalJSON for MediaMetadata.Series (handles both object and array). |
 | `libraries.go` | GetLibraries, GetPersonalized, GetLibraryItems (paginated), GetLibraryItemsBatch (10 concurrent), GetLibrarySeries, GetSeriesContents, FilterAudioLibraries (sample-based audio detection) |
-| `playback.go` | StartPlaySession, StartEpisodePlaySession, SyncSession, CloseSession |
-| `progress.go`, `me.go` | UpdateProgress, GetMediaProgress (with bookmarks), UpdateEpisodeProgress, GetMe (user + all media progress) |
+| `playback.go` | StartPlaySession, StartEpisodePlaySession, SyncSession, CloseSession, CloseSessionNoSync (sends no position) |
+| `progress.go`, `me.go` | UpdateProgress, UpdatePosition (no `isFinished` key), GetMediaProgress (with bookmarks), UpdateEpisodeProgress, GetMe (user + all media progress) |
 | `bookmarks.go` | CreateBookmark, UpdateBookmark, DeleteBookmark, GetBookmarks |
 
 ### Data (internal/db/)
@@ -140,8 +140,10 @@ Styles from theme config. Format helpers (timestamps, durations). Components: He
 - Skeleton loading with generation-gated reveal delays (home: 500ms, library: 150ms)
 - Cache-first data loading through `internal/cache.Client`: home and library views hit the persistent cache first; misses trigger a background rebuild
 - `q` always quits, `esc`/left goes back (never quits). `h`/`l` seek only, arrows navigate only
+- A quit (`q` or ctrl+c) returns only `tea.Quit` and keeps the session in the model. `Cleanup` sends the last position, and `main` waits for it. Bubble Tea does not wait for a command at a quit, so a quit must not send the position in a command.
 - `ctrl+p` opens the global command palette from any screen
 - Duration from ABS takes precedence over mpv-reported duration (streaming content reports 0)
+- The listening time (`timeListened`) is the time of the clock between two position ticks of a player that plays (`trackListeningTime`). It is not the change of the position: at the speed 2.0, the position moves two seconds in each second. A test moves the clock with `Model.now`.
 - pine finds the end of a track through `errors.Is(err, player.ErrClosed)` on a position tick. spruce uses `PlayerEndMsg`, and a tick with `ErrClosed` can arrive before it. Do not match the text of a player error.
 
 ## Testing

internal/abs/playback.go

@@ -89,7 +89,8 @@ func (c *Client) SyncSession(ctx context.Context, sessionID string, currentTime,
 	return nil
 }
 
-// CloseSession closes an active playback session.
+// CloseSession closes an active playback session. The server stores the
+// position and the listening time before it closes the session.
 func (c *Client) CloseSession(ctx context.Context, sessionID string, currentTime, timeListened float64) error {
 	path := fmt.Sprintf("/api/session/%s/close", sessionID)
 	body := syncSessionRequest{
@@ -103,3 +104,15 @@ func (c *Client) CloseSession(ctx context.Context, sessionID string, currentTime
 	}
 	return nil
 }
+
+// CloseSessionNoSync closes an active playback session and sends no position,
+// so the server keeps the progress that it has.
+func (c *Client) CloseSessionNoSync(ctx context.Context, sessionID string) error {
+	path := fmt.Sprintf("/api/session/%s/close", sessionID)
+	logger.Debug("closing playback session with no sync", "sessionID", sessionID)
+	_, err := c.do(ctx, http.MethodPost, path, nil)
+	if err != nil {
+		return fmt.Errorf("close session: %w", err)
+	}
+	return nil
+}

internal/abs/playback_test.go

@@ -247,3 +247,26 @@ func TestCloseSessionHTTP(t *testing.T) {
 		t.Errorf("timeListened = %f, want 180.0", body.TimeListened)
 	}
 }
+
+func TestCloseSessionNoSyncHTTP(t *testing.T) {
+	var capturedBody []byte
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/api/session/session-abc-123/close" {
+			t.Errorf("path = %q, want /api/session/session-abc-123/close", r.URL.Path)
+		}
+		if r.Method != http.MethodPost {
+			t.Errorf("method = %q, want POST", r.Method)
+		}
+		capturedBody, _ = io.ReadAll(r.Body)
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer srv.Close()
+
+	c := NewClient(srv.URL, "tok")
+	if err := c.CloseSessionNoSync(context.Background(), "session-abc-123"); err != nil {
+		t.Fatalf("CloseSessionNoSync() error: %v", err)
+	}
+	if len(capturedBody) != 0 {
+		t.Errorf("body = %q, want no body", capturedBody)
+	}
+}

internal/abs/progress.go

@@ -8,10 +8,13 @@ import (
 )
 
 // updateProgressRequest is the body for PATCH /api/me/progress/{id}.
+// The server reads "isFinished": false as "mark as not finished": it removes
+// the finished mark and sets the position to 0. With no isFinished key, the
+// server sets or keeps the mark by its own rule for the position.
 type updateProgressRequest struct {
 	CurrentTime float64 `json:"currentTime"`
 	Progress    float64 `json:"progress"`
-	IsFinished  bool    `json:"isFinished"`
+	IsFinished  *bool   `json:"isFinished,omitempty"`
 }
 
 // UpdateProgress updates the user's progress on a library item.
@@ -20,7 +23,7 @@ func (c *Client) UpdateProgress(ctx context.Context, itemID string, currentTime,
 	body := updateProgressRequest{
 		CurrentTime: currentTime,
 		Progress:    progress,
-		IsFinished:  isFinished,
+		IsFinished:  &isFinished,
 	}
 	_, err := c.do(ctx, http.MethodPatch, path, body)
 	if err != nil {
@@ -29,6 +32,24 @@ func (c *Client) UpdateProgress(ctx context.Context, itemID string, currentTime,
 	return nil
 }
 
+// UpdatePosition updates the user's position in a library item, or in a
+// podcast episode if episodeID is not empty. It sends no isFinished key.
+func (c *Client) UpdatePosition(ctx context.Context, itemID, episodeID string, currentTime, progress float64) error {
+	path := fmt.Sprintf("/api/me/progress/%s", itemID)
+	if episodeID != "" {
+		path += "/" + episodeID
+	}
+	body := updateProgressRequest{
+		CurrentTime: currentTime,
+		Progress:    progress,
+	}
+	_, err := c.do(ctx, http.MethodPatch, path, body)
+	if err != nil {
+		return fmt.Errorf("update position: %w", err)
+	}
+	return nil
+}
+
 // GetMediaProgress returns the user's media progress (including bookmarks)
 // for a specific library item.
 func (c *Client) GetMediaProgress(ctx context.Context, itemID string) (*MediaProgressWithBookmarks, error) {
@@ -50,7 +71,7 @@ func (c *Client) UpdateEpisodeProgress(ctx context.Context, itemID, episodeID st
 	body := updateProgressRequest{
 		CurrentTime: currentTime,
 		Progress:    progress,
-		IsFinished:  isFinished,
+		IsFinished:  &isFinished,
 	}
 	_, err := c.do(ctx, http.MethodPatch, path, body)
 	if err != nil {

internal/abs/progress_test.go

@@ -6,6 +6,7 @@ import (
 	"io"
 	"net/http"
 	"net/http/httptest"
+	"reflect"
 	"testing"
 )
 
@@ -78,6 +79,47 @@ func TestUpdateProgressFinishedHTTP(t *testing.T) {
 	}
 }
 
+func TestUpdatePositionHTTP(t *testing.T) {
+	tests := []struct {
+		name      string
+		episodeID string
+		wantPath  string
+	}{
+		{name: "book", wantPath: "/api/me/progress/li-001"},
+		{name: "episode", episodeID: "ep-001", wantPath: "/api/me/progress/li-001/ep-001"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var capturedBody []byte
+			srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				if r.URL.Path != tt.wantPath {
+					t.Errorf("path = %q, want %s", r.URL.Path, tt.wantPath)
+				}
+				if r.Method != http.MethodPatch {
+					t.Errorf("method = %q, want PATCH", r.Method)
+				}
+				capturedBody, _ = io.ReadAll(r.Body)
+				w.WriteHeader(http.StatusOK)
+			}))
+			defer srv.Close()
+
+			c := NewClient(srv.URL, "tok")
+			if err := c.UpdatePosition(context.Background(), "li-001", tt.episodeID, 500.5, 0.45); err != nil {
+				t.Fatalf("UpdatePosition() error: %v", err)
+			}
+
+			var body map[string]any
+			if err := json.Unmarshal(capturedBody, &body); err != nil {
+				t.Fatalf("failed to unmarshal request body: %v", err)
+			}
+			want := map[string]any{"currentTime": 500.5, "progress": 0.45}
+			if !reflect.DeepEqual(body, want) {
+				t.Errorf("body = %v, want %v (no isFinished key)", body, want)
+			}
+		})
+	}
+}
+
 func TestGetMediaProgressHTTP(t *testing.T) {
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		if r.URL.Path != "/api/me/progress/li-001" {

internal/app/e2e_test.go

@@ -245,12 +245,12 @@ func TestE2E_DetailToPlayback(t *testing.T) {
 	// The tick commands use time.After so they'll timeout in feedCmd — that's fine
 	m = feedCmdChain(m, cmd, 3)
 
-	// Simulate position updates
+	// Simulate position updates: the first tick of the player, and then 10
+	// seconds of play
 	mp.mu.Lock()
 	mp.position = 52.0
 	mp.mu.Unlock()
-	res, _ := m.Update(player.PositionMsg{Position: 52.0, Duration: 3600.0, Paused: false, Generation: 1})
-	m = res.(Model)
+	m = listen(playTick(m, 0, 42.0), 52.0)
 
 	if m.player.Position != 52.0 {
 		t.Errorf("position = %f, want 52.0", m.player.Position)
@@ -263,7 +263,7 @@ func TestE2E_DetailToPlayback(t *testing.T) {
 	assertViewContains(t, m, "The Great Gatsby")
 
 	// Trigger sync
-	res, cmd = m.Update(SyncTickMsg{})
+	res, cmd := m.Update(SyncTickMsg{})
 	m = res.(Model)
 	executeBatchCmds(cmd)
 
@@ -1048,11 +1048,10 @@ func TestE2E_FullJourney(t *testing.T) {
 	mp.mu.Lock()
 	mp.position = 60.0
 	mp.mu.Unlock()
-	res, _ := m.Update(player.PositionMsg{Position: 60.0, Duration: 3600.0, Paused: false, Generation: 1})
-	m = res.(Model)
+	m = playTick(playTick(m, 0, 59.0), 1, 60.0)
 
-	if m.timeListened == 0 {
-		t.Error("timeListened should be > 0 after position update")
+	if m.timeListened != 1.0 {
+		t.Errorf("timeListened = %f, want 1.0 after one second of play", m.timeListened)
 	}
 
 	// === Phase 7: Create bookmark ===
@@ -1062,7 +1061,7 @@ func TestE2E_FullJourney(t *testing.T) {
 	assertAPICallMade(t, log, "POST", "/api/me/item/")
 
 	// === Phase 8: Sync tick ===
-	res, cmd = m.Update(SyncTickMsg{})
+	res, cmd := m.Update(SyncTickMsg{})
 	m = res.(Model)
 	executeBatchCmds(cmd)
 	assertAPICallMade(t, log, "POST", "/api/session/sess-e2e/sync")
@@ -1086,13 +1085,13 @@ func TestE2E_FullJourney(t *testing.T) {
 	}
 
 	// === Phase 11: Quit from Home (stops playback and exits) ===
-	_, cmd = m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'q'}})
+	res, cmd = m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'q'}})
 	if cmd == nil {
 		t.Fatal("'q' on Home with empty back stack should quit")
 	}
 
-	// Should batch stop commands with quit
-	executeBatchCmds(cmd)
+	// main calls Cleanup on the final model, which closes the session.
+	res.(Model).Cleanup()
 	assertAPICallMade(t, log, "POST", "/api/session/sess-e2e/close")
 
 	// === Verify API call sequence ===
@@ -1669,13 +1668,13 @@ func TestE2E_QQuitsAndStopsPlayback(t *testing.T) {
 	m.player.Duration = 3600.0
 
 	// Press q — should quit and stop playback
-	_, cmd := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'q'}})
+	res, cmd := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'q'}})
 	if cmd == nil {
 		t.Fatal("q should return a command")
 	}
 
-	// Execute the batch — should include stop + quit
-	executeBatchCmds(cmd)
+	// main calls Cleanup on the final model, which closes the session.
+	res.(Model).Cleanup()
 
 	// Verify close session API was called
 	assertAPICallMade(t, log, "POST", "/api/session/sess-e2e/close")

internal/app/messages.go

@@ -62,6 +62,7 @@ type PlaySessionData struct {
 	ItemID           string
 	EpisodeID        string
 	CurrentTime      float64
+	SyncedTime       float64 // book position that the server has
 	Duration         float64
 	Title            string
 	Authors          []string
@@ -81,6 +82,17 @@ type QueueEntry struct {
 // SyncTickMsg fires every 30 seconds to sync playback progress with ABS.
 type SyncTickMsg struct{}
 
+// SessionSyncedMsg carries the result of a sync of the playback session.
+// NewSessionID is the session that pine opened because the server did not
+// have the session SessionID.
+type SessionSyncedMsg struct {
+	SessionID    string
+	NewSessionID string
+	CurrentTime  float64
+	TimeListened float64
+	Err          error
+}
+
 // PlaybackErrorMsg carries an error from the playback lifecycle.
 type PlaybackErrorMsg struct {
 	Err error

internal/app/model.go

@@ -55,6 +55,7 @@ type Model struct {
 	itemID                   string
 	episodeID                string
 	timeListened             float64
+	lastPlayingTick          time.Time
 	lastSyncPos              float64
 	playGeneration           uint64
 	chapters                 []abs.Chapter
@@ -100,6 +101,9 @@ type Model struct {
 	lastPlayedItemID  string
 	lastPlayedAuthors []string
 	currentAuthors    []string
+
+	// now replaces time.Now in a test.
+	now func() time.Time
 }
 
 // New creates a new root model. If client is non-nil (authenticated),
@@ -636,6 +640,9 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 		}
 		return m, cmd
 
+	case SessionSyncedMsg:
+		return m.handleSessionSynced(msg), nil
+
 	case player.PlayerLaunchErrMsg:
 		if msg.Generation != m.playGeneration {
 			logger.Debug("ignoring stale player launch error", "msgGen", msg.Generation, "currentGen", m.playGeneration)
@@ -740,11 +747,9 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 		return m, nil
 
 	case tea.KeyMsg:
+		// A quit sends no command for the session: Cleanup sends the last
+		// position. See the quit with q below.
 		if msg.Type == tea.KeyCtrlC {
-			if m.isPlaying() {
-				_, stopCmd := m.stopPlayback()
-				return m, tea.Batch(stopCmd, tea.Quit)
-			}
 			return m, tea.Quit
 		}
 		if !m.isFiltering() && key.Matches(msg, m.keys.Help) {
@@ -824,12 +829,11 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 			m.toggleCaptions()
 			return m, nil
 		}
-		// Global quit: q always quits the app.
+		// Global quit: q always quits the app. The session stays in the model:
+		// main calls Cleanup on the final model, and Cleanup waits until the
+		// server has the last position. The program does not wait for a
+		// command, so the request of a command can be lost.
 		if !m.isFiltering() && m.screen != ScreenLogin && key.Matches(msg, m.keys.Quit) {
-			if m.isPlaying() {
-				m, stopCmd := m.stopPlayback()
-				return m, tea.Batch(stopCmd, tea.Quit)
-			}
 			return m, tea.Quit
 		}
 		// Global back: esc/left goes back but never quits.