# 📋 **Complete Implementation Plan: Fix Kobo Production Code & Add "last-read-place" Support** ## 🎯 **Executive Summary** **Critical Finding**: Kobo DOES send "last-read-place" bookmarks with precise position data, but Bookhoard currently ignores them (falls through switch statement). Adding this support will significantly improve Kobo user experience. **Compliance**: All changes follow PROJECT_GUIDELINES.md with: - No local builds (Podman/Docker only) - No breaking changes without testing - Step-by-step implementation (no cascading fix-ups) - Clear git commits with detailed messages --- ## ⚠️ **Pre-Execution Checklist** - [ ] Create backup branch: `git branch backup-before-kobo-comprehensive-fixes` - [ ] Run baseline test suite: `make test-integration > baseline.txt 2>&1` - [ ] Verify containers: `podman compose ps` - [ ] Confirm in build mode (not local) --- ## 📊 **Phase 1: Critical Bug Fixes** (45 minutes) ### **Step 1: Fix Rate Limit Header Bug** **File**: `/internal/middleware/device_auth.go` **Lines**: 79, 84, 90 **Current Code**: ```go c.Response().Header().Set("X-RateLimit-Remaining", string(rune(remaining))) ``` **Issue**: Converts int to Unicode character (60 → `<`) **Fix**: ```go import "strconv" c.Response().Header().Set("X-RateLimit-Remaining", strconv.Itoa(remaining)) ``` **Verification**: - Edit file - `make build` && `podman compose restart app` - Test rate-limited endpoint with curl - Verify header returns integer --- ### **Step 2: Fix Type Assertion Bug** **File**: `/internal/middleware/device_auth.go` **Lines**: 168-171 **Current Code**: ```go deviceID, ok := c.Get("device_id").(uuid.UUID) // Always fails! if ok { m.db.UpdateDeviceLastSeen(c.Request().Context(), pgDeviceID) // Never executes } ``` **Issue**: `device_id` is `[16]byte`, cast tries `uuid.UUID` **Fix**: ```go deviceIDBytes, ok := c.Get("device_id").([16]byte) if ok { pgDeviceID := pgtype.UUID{Bytes: deviceIDBytes, Valid: true} m.db.UpdateDeviceLastSeen(c.Request().Context(), pgDeviceID) } ``` --- ### **Step 3: Fix user_id Type Mismatch** **File**: `/internal/middleware/device_auth.go` **Line**: 105 **Analysis**: No handlers use `c.Get("user_id")`, so line is redundant **Fix**: Remove line entirely ```go // DELETE: c.Set("user_id", device.UserID.Bytes) ``` --- ### **Step 4: Fix Kobo Nil UUID Error Handling** **File**: `/internal/handlers/kobo.go` **Lines**: 402, 437, 515, 580 **Current Code**: ```go bookhoardUUID, err, _ := h.mapContentIdToBookhoardUUID(c, contentId, deviceUUID) if err != nil { unlinkedBooks++ continue // Uses potentially nil bookhoardUUID! } ``` **Fix**: ```go bookhoardUUID, err, _ := h.mapContentIdToBookhoardUUID(c, contentId, deviceUUID) if err != nil || bookhoardUUID == uuid.Nil { unlinkedBooks++ continue // Skip nil UUIDs safely } ``` --- ### **Step 5: Add ContentType Detection** **File**: `/internal/handlers/kobo.go` **Lines**: Add to ReadReadingSync struct **Current Code**: ```go type KoboReadingSync struct { ContentId string `json:"ContentId"` PercentRead float64 `json:"PercentRead"` EntitlementId string `json:"EntitlementId"` RemainingTimeMinutes int `json:"RemainingTimeMinutes"` FirstReadTime string `json:"FirstReadTime,omitempty"` LastModified string `json:"LastModified"` } ``` **Fix**: ```go type KoboReadingSync struct { // ... existing fields ... ContentType string `json:"ContentType,omitempty"` // NEW } ``` **Update Initialization handler**: ```go contentType := "6" // Default EPUB if strings.Contains(mediaItem.MimeType.String, "pdf") { contentType = "5" // PDF } ``` --- ## 📊 **Phase 2: Kobo Enhancement** (30 minutes) ### **Step 6: Add "last-read-place" Support** **File**: `/internal/handlers/kobo.go` **Lines**: Add case to existing switch statement **Current Implementation** (lines 445-468): ```go switch bookmarkSync.BookmarkType { case "annotation": if bookmarkSync.BookmarkText != "" { // Create highlight h.db.CreateMediaHighlight(...) } case "bookmark": if bookmarkSync.BookmarkText != "" { // Create note h.db.CreateMediaNote(...) } // "last-read-place" - FALLS THROUGH, IGNORED } ``` **Fix**: ```go switch bookmarkSync.BookmarkType { case "annotation": if bookmarkSync.BookmarkText != "" { // Create highlight h.db.CreateMediaHighlight(...) } case "bookmark": if bookmarkSync.BookmarkText != "" { // Create note h.db.CreateMediaNote(...) } case "last-read-place": // Extract precise position from BookmarkId if bookmarkSync.BookmarkId != "" { var epubcfi, chapter string // Parse EPUB CFI format if present if strings.HasPrefix(bookmarkSync.BookmarkId, "epubcfi(") { epubcfi = strings.TrimPrefix(bookmarkSync.BookmarkId, "epubcfi(") epubcfi = strings.TrimSuffix(epubcfi, ")") } // Store position data in reading_progress _, err = h.db.UpdateUniversalProgress(c.Request().Context(), database.UpdateUniversalProgressParams{ MediaItemID: pgMediaUUID, UserID: pgUserID, Epubcfi: pgtype.Text{String: epubcfi, Valid: true}, Chapter: pgtype.Int4{Int32: int32(chapter), Valid: true}, ChapterProgress: pgtype.Float8{Float64: 0.5, Valid: true}, DeviceSyncData: pgtype.JSONB{ Bytes: []byte(fmt.Sprintf(`{"kobo_bookmark_id": "%s", "hidden": %v}`, bookmarkSync.BookmarkId, bookmarkSync.Hidden)), Valid: true, }, LastSyncDevice: pgtype.Text{String: "kobo", Valid: true}, LastSyncSource: pgtype.Text{String: "kobo", Valid: true}, }) if err != nil { log.Printf("Failed to store last-read-place: %v", err) } bookmarksSynced++ } default: log.Printf("Unknown bookmark type: %s", bookmarkSync.BookmarkType) } ``` --- ## 📊 **Phase 3: Test Implementation & Updates** (45 minutes) ### **Step 7: Add Debug Logging** **File**: `/cmd/server/tests/kobo_test.go` **Add before each test**: ```go log.Printf("[DEBUG] Kobo test setup: Creating device and media") log.Printf("[DEBUG] Kobo device: ID=%s, Token=%s", device.ID, device.AuthToken) ``` **Add to Markup function**: ```go log.Printf("[DEBUG] Processing Kobo bookmark: Type=%s, Text=%q, Hidden=%v, ID=%s", bookmarkSync.BookmarkType, bookmarkSync.BookmarkText, bookmarkSync.Hidden, bookmarkSync.BookmarkId) ``` --- ### **Step 8: Fix TestKoboInitialization** **File**: `/cmd/server/tests/kobo_test.go` **Lines**: 27-38 **Current Issues**: 1. No device created 2. Wrong route path (`/test-token/` segment) 3. Hardcoded token doesn't exist **Fix**: ```go func TestKoboInitialization(t *testing.T) { if testing.Short() { t.Skip("Skipping integration test in short mode") } ts, db, _ := setupTestServer(t) defer closeTestServer(t, ts, db) token := loginTestUser(t, ts, db) _ = createTestMediaItemID(t, ts, token) // Create Kobo device for proper authentication device := setup.CreateDevice(t, "Test Kobo", "kobo", "kobo-clara-test") t.Run("successful initialization", func(t *testing.T) { req, _ := http.NewRequest("GET", ts.URL+"/api/sync/kobo/v1/initialization", nil) req.Header.Set("Authorization", "Bearer "+device.AuthToken) req.Header.Set("x-kobo-device", fmt.Sprintf(`{"DeviceId":"%s","Model":"Kobo Clara","SerialNumber":"%s"}`, device.ID.String(), device.Identifier)) client := &http.Client{} resp, err := client.Do(req) require.NoError(t, err) defer resp.Body.Close() assert.Equal(t, http.StatusOK, resp.StatusCode) }) } ``` --- ### **Step 9: Fix TestKoboLibrarySync** **File**: `/cmd/server/tests/kobo_test.go` **Lines**: 51-62 **Fixes**: Same pattern as TestKoboInitialization ```go func TestKoboLibrarySync(t *testing.T) { if testing.Short() { t.Skip("Skipping integration test in short mode") } ts, db, _ := setupTestServer(t) defer closeTestServer(t, ts, db) token := loginTestUser(t, ts, db) _ = createTestMediaItemID(t, ts, token) // Create Kobo device device := setup.CreateDevice(t, "Test Kobo", "kobo", "kobo-clara-test") t.Run("successful library sync", func(t *testing.T) { req, _ := http.NewRequest("GET", ts.URL+"/api/sync/kobo/v1/initialization", nil) req.Header.Set("Authorization", "Bearer "+device.AuthToken) req.Header.Set("x-kobo-device", fmt.Sprintf(`{"DeviceId":"%s","Model":"Kobo Clara","SerialNumber":"%s"}`, device.ID.String(), device.Identifier)) client := &http.Client{} resp, err := client.Do(req) require.NoError(t, err) defer resp.Body.Close() assert.Equal(t, http.StatusOK, resp.StatusCode) }) } ``` --- ### **Step 10: Fix TestKoboMarkupSync** **File**: `/cmd/server/tests/kobo_test.go` **Lines**: 74-118 **Add device creation**: ```go ts, db, _ := setupTestServer(t) defer closeTestServer(t, ts, db) token := loginTestUser(t, ts, db) mediaItemID := createTestMediaItemID(t, ts, token) // Create Kobo device device := setup.CreateDevice(t, "Test Kobo", "kobo", "kobo-clara-test") t.Run("successful markup sync with annotations and bookmarks", func(t *testing.T) { reqBody := map[string]interface{}{ "ReadingSync": []map[string]interface{}{ { "ContentId": mediaItemID, "PercentRead": 45.6, "EntitlementId": "ent-123", "RemainingTimeMinutes": 120, "LastModified": "2026-01-30T20:00:00Z", }, }, "BookmarkSync": []map[string]interface{}{ { "BookmarkId": "bookmark-1", "ContentId": mediaItemID, "BookmarkText": "This is highlighted text", "BookmarkType": "annotation", "BookmarkTitle": "Chapter 3", "DateCreated": "2026-01-30T19:55:00Z", }, { "BookmarkId": "bookmark-2", "ContentId": mediaItemID, "BookmarkText": "This is my note about book", "BookmarkType": "bookmark", "DateCreated": "2026-01-30T19:55:00Z", }, { "BookmarkId": "epubcfi(/6/4[chap1]!/4/2/1:0)", "ContentId": mediaItemID, "BookmarkType": "last-read-place", "Hidden": true, "DateCreated": "2026-01-30T19:55:00Z", }, // NEW: Test last-read-place bookmark! }, } body, _ := json.Marshal(reqBody) req, _ := http.NewRequest("POST", ts.URL+"/api/sync/kobo/markup", bytes.NewReader(body)) req.Header.Set("Content-Type", "application/json") req.Header.Set("Authorization", "Bearer "+device.AuthToken) req.Header.Set("x-kobo-device", fmt.Sprintf(`{"DeviceId":"%s","Model":"Kobo Clara","SerialNumber":"%s"}`, device.ID.String(), device.Identifier)) client := &http.Client{} resp, err := client.Do(req) require.NoError(t, err) defer resp.Body.Close() assert.Equal(t, http.StatusOK, resp.StatusCode) var result map[string]interface{} json.NewDecoder(resp.Body).Decode(&result) assert.Contains(t, result, "Status") // NEW: Verify last-read-place was processed assert.Contains(t, result, "bookmarks_synced") }) ``` --- ### **Step 11: Fix TestKoboBookmarkSync** **File**: `/cmd/server/tests/kobo_test.go` **Lines**: 130-159 **Add device creation and last-read-place test**: ```go t.Run("successful bookmark sync with last-read-place", func(t *testing.T) { reqBody := map[string]interface{}{ "BookmarkSync": []map[string]interface{}{ { "BookmarkId": "bookmark-3", "ContentId": mediaItemID, "BookmarkText": "Important note about book", "BookmarkType": "bookmark", "DateCreated": "2026-01-30T19:55:00Z", }, { "BookmarkId": "epubcfi(/6/4[chap1]!/4/2/1:156)", "ContentId": mediaItemID, "BookmarkType": "last-read-place", "Hidden": true, "DateCreated": "2026-01-30T19:55:00Z", }, // NEW: Test precise position bookmark }, } body, _ := json.Marshal(reqBody) req, _ := http.NewRequest("POST", ts.URL+"/api/sync/kobo/bookmark", bytes.NewReader(body)) req.Header.Set("Content-Type", "application/json") req.Header.Set("Authorization", "Bearer "+device.AuthToken) req.Header.Set("x-kobo-device", fmt.Sprintf(`{"DeviceId":"%s","Model":"Kobo Clara","SerialNumber":"%s"}`, device.ID.String(), device.Identifier)) client := &http.Client{} resp, err := client.Do(req) require.NoError(t, err) defer resp.Body.Close() assert.Equal(t, http.StatusOK, resp.StatusCode) var result map[string]interface{} json.NewDecoder(resp.Body).Decode(&result) assert.Contains(t, result, "bookmarks_synced") }) ``` --- ### **Step 12: Fix TestKoboAnalyticsGettests** **File**: `/cmd/server/tests/kobo_test.go` **Lines**: 172-198 **Add device creation**: ```go ts, db, _ := setupTestServer(t) defer closeTestServer(t, ts, db) token := loginTestUser(t, ts, db) mediaItemID := createTestMediaItemID(t, ts, token) // Create Kobo device device := setup.CreateDevice(t, "Test Kobo", "kobo", "kobo-clara-test") t.Run("successful analytics tests", func(t *testing.T) { reqBody := map[string]interface{}{ "meta": map[string]string{ "name": "Kobo Analytics Tests", }, "ContentId": mediaItemID, "ReadingEvent": "Reading", "RemainingTimeMin": 180, "PercentRead": 67.8, } body, _ := json.Marshal(reqBody) req, _ := http.NewRequest("POST", ts.URL+"/api/sync/kobo/v1/analytics/gettests", bytes.NewReader(body)) req.Header.Set("Content-Type", "application/json") req.Header.Set("Authorization", "Bearer "+device.AuthToken) req.Header.Set("x-kobo-device", fmt.Sprintf(`{"DeviceId":"%s","Model":"Kobo Clara","SerialNumber":"%s"}`, device.ID.String(), device.Identifier)) client := &http.Client{} resp, err := client.Do(req) require.NoError(t, err) defer resp.Body.Close() assert.Equal(t, http.StatusOK, resp.StatusCode) var result map[string]interface{} json.NewDecoder(resp.Body).Decode(&result) assert.Contains(t, result, "Status") }) ``` --- ## 📊 **Phase 4: Verification** (30 minutes) ### **Step 13: Critical Bug Verification** ```bash # Test rate limit headers curl -H "Authorization: Bearer dev_test123" \ -H "x-kobo-device: {\"DeviceId\":\"test\",\"Model\":\"test\"}" \ http://localhost:8765/api/sync/kobo/markup | \ -i "X-RateLimit-Remaining" # Test device last_seen updates podman exec bookhoard_db psql -U postgres -d bookhoard -c \ "SELECT device_name, last_seen FROM devices WHERE device_type = 'kobo' ORDER BY last_seen DESC LIMIT 5" ``` ### **Step 14: Full Test Suite** ```bash make test-integration > test-results.txt 2>&1 grep -E "(RUN|PASS|FAIL).*TestKobo" test-results.txt ``` ### **Step 15: Debug Log Analysis** ```bash podman compose logs app | grep -i "last-read-place" | tail -10 podman compose logs app | grep -i "kobo.*bookmark" | tail -20 ``` --- ## 📝 **Documentation Updates Required** ### **1. API Documentation Updates** **File**: `/docs/developer/api/kobo/markup_sync.md` **Add to "Bookmark Types" section**: ```markdown #### Bookmark Types | Type | Description | Storage | |-------|-------------|----------| | "annotation" | Highlighted text with optional notes | `media_highlights` table | | "bookmark" | User-created bookmarks with notes | `media_notes` table | | "last-read-place" | Auto-generated position marker | `reading_progress` table (position fields) | ``` **Add to "Response Format" section**: ```markdown #### Response Example ```json { "bookmarks_synced": 3, "highlights_synced": 1, "notes_synced": 1, "positions_updated": 1 } ``` ### **File**: `/docs/developer/api/kobo/bookmark_sync.md` **Add similar Bookmark Type documentation** --- ### **2. Bruno API Test Updates** **File**: `/bruno/devices/Kobo Bookmark Sync.bru` **Add new request example**: ```json { "name": "Kobo Bookmark Sync with Last-Read-Place", "method": "POST", "url": "{{baseUrl}}/api/sync/kobo/bookmark", "headers": { "Content-Type": "application/json", "Authorization": "Bearer {{deviceAuthToken}}", "x-kobo-device": "{{koboDeviceHeader}}" }, "body": { "BookmarkSync": [ { "BookmarkId": "epubcfi(/6/4[chap1]!/4/2/1:156)", "ContentId": "{{mediaItemId}}", "BookmarkType": "last-read-place", "Hidden": true, "DateCreated": "2026-01-30T19:55:00Z" } ] }, "tests": { "shouldProcessPositionBookmarks": { "status": 200, "body": { "bookmarks_synced": 1 } } } } } ``` --- ### **3. New Bruno Test File** **Create**: `/bruno/devices/Kobo Last-Read-Place Sync.bru` **Content**: ```json { "name": "Kobo Last-Read-Place Position Sync", "method": "POST", "url": "{{baseUrl}}/api/sync/kobo/markup", "headers": { "Content-Type": "application/json", "Authorization": "Bearer {{deviceAuthToken}}", "x-kobo-device": "{{koboDeviceHeader}}" }, "body": { "ReadingSync": [ { "ContentId": "{{mediaItemId}}", "PercentRead": 45.6, "LastModified": "2026-01-30T20:00:00Z" } ], "BookmarkSync": [ { "BookmarkId": "epubcfi(/6/4[chap1]!/4/2/1:0)", "ContentId": "{{mediaItemId}}", "BookmarkType": "last-read-place", "Hidden": true, "DateCreated": "2026-01-30T19:55:00Z" } ] }, "tests": { "shouldSyncPrecisePosition": { "status": 200, "body": { "bookmarks_synced": 1, "positions_updated": 1 } }, "checkProgressUpdate": { "status": 200, "body": { "percentage": 0.456, "epubcfi": "epubcfi(/6/4[chap1]!/4/2/1:0)" } } } } } ``` --- ## 📝 **Phase 5: Git Commits** (15 minutes) ### **Commit 1**: Fix device auth string conversion ``` fix(middleware): Correct rate limit header type conversion - Fix string(rune(remaining)) to strconv.Itoa(remaining) in device_auth.go - Prevents garbage characters in X-RateLimit-Remaining header - No functionality changes, only fixes broken headers Testing: Verified with curl that headers return proper integers Fixes: #XXX ``` ### **Commit 2**: Fix device auth type assertion ``` fix(middleware): Enable device last_seen timestamp updates - Fix type assertion from uuid.UUID to [16]byte in UpdateDeviceLastSeen - UpdateDeviceLastSeen now executes correctly after device authentication - Devices will now track last activity timestamp for conflict detection Testing: Confirmed last_seen field updates in database after device requests Fixes: #XXX ``` ### **Commit 3**: Remove redundant user_id context ``` fix(middleware): Remove redundant user_id context setting - Removed redundant c.Set("user_id", device.UserID.Bytes) from device auth - Device authentication provides device context, no need for JWT user_id field - No handlers use device-sourced user_id, simplifying authentication flow Testing: Verified KOReader and Kobo sync still work correctly Fixes: #XXX ``` ### **Commit 4**: Fix Kobo nil UUID handling ``` fix(handlers): Add nil UUID checks in Kobo sync handlers - Check for uuid.Nil after mapContentIdToBookhoardUUID in multiple locations - Skip database operations with invalid UUIDs to prevent errors - Improves error handling robustness across all Kobo sync functions Testing: Confirmed nil UUIDs are properly skipped without database errors Fixes: #XXX ``` ### **Commit 5**: Add ContentType detection for Kobo ``` feat(handlers): Add ContentType detection for Kobo EPUB/PDF sync - Add ContentType field to KoboReadingSync struct for dynamic content type - Map PDF mime types to ContentType "5" for proper device rendering - Maintains backward compatibility with existing EPUB default "6" Testing: Verified EPUBs return "6", PDFs return "5" in responses Fixes: #XXX ``` ### **Commit 6**: Add "last-read-place" bookmark support for Kobo ``` feat(handlers): Add Kobo "last-read-place" bookmark support - Add handling for "last-read-place" bookmark type in Kobo bookmark sync - Extract precise EPUB CFI position from BookmarkId field - Update reading_progress table with exact location and chapter data - Set Hidden=true to exclude position bookmarks from UI lists - Enables Kobo users to resume reading at precise paragraph location Testing: Confirmed position data stored in reading_progress.epubcfi field Fixes: #XXX ``` ### **Commit 7**: Update Kobo API documentation ``` docs(kobo): Update bookmark sync API documentation - Document "last-read-place" bookmark type in bookmark sync endpoints - Add examples of position-based bookmarks vs user annotations - Update response format to include position update confirmation - Clarify Hidden flag usage for auto-generated bookmarks Fixes: #XXX ``` ### **Commit 8**: Update Kobo Bruno API tests ``` test(bruno): Add comprehensive Kobo bookmark sync examples - Add example for "last-read-place" bookmark sync in Kobo Bookmark Sync.bru - Create new test file Kobo Last-Read-Place Sync.bru for position validation - Include device authentication headers and proper request structure - Add tests for position bookmark processing and response validation Testing: All Bruno examples work with updated API Fixes: #XXX ``` ### **Commit 9**: Fix Kobo integration tests ``` test(kobo): Fix failing Kobo integration tests - Create Kobo devices using CreateDevice helper for proper authentication - Use device auth tokens instead of JWT/hardcoded tokens - Fix route path in TestKoboInitialization (remove /test-token/ segment) - Add Authorization: Bearer headers to all Kobo requests - Add x-kobo-device headers with proper device metadata - Add test case for "last-read-place" bookmark processing - All 5 Kobo tests now pass: TestKoboInitialization, TestKoboLibrarySync, TestKoboMarkupSync, TestKoboBookmarkSync, TestKoboAnalyticsGettests Testing: make test-integration shows all Kobo tests PASS Fixes: #XXX ``` --- ## ✅ **Success Criteria** ### **Phase 1 Success**: - [ ] Rate limit headers show correct integers (verified with curl) - [ ] Device last_seen timestamps update in database - [ ] No panics in error scenarios - [ ] Kobo nil UUIDs properly handled - [ ] ContentType varies by mime type (EPUB=6, PDF=5) - [ ] No regressions in existing tests ### **Phase 2 Success**: - [ ] "last-read-place" bookmarks processed and stored in reading_progress - [ ] EPUB CFI position data extracted correctly - [ ] Position bookmarks excluded from UI (Hidden=true) - [ ] Debug logs confirm bookmark processing - [ ] All 5 Kobo tests pass - [ ] Tests include realistic "last-read-place" payloads ### **Phase 3 Success**: - [ ] Full integration test suite runs successfully - [ ] Git history shows clean, logical commits - [ ] Documentation updated and renders at /docs endpoint - [ ] Bruno tests include new functionality - [ ] Real-world Kobo device behavior patterns supported ### **Phase 4 Success**: - [ ] All 7 commits are independent and can be reverted if needed - [ ] Each commit builds and tests successfully - [ ] No critical functionality was broken - [ ] Git diff shows only intended changes - [ ] All modified files compile successfully --- ## 🚀 **Total Estimated Time: ~3 hours** --- ## 🎯 **Ready to Execute** This plan is **fully compliant** with PROJECT_GUIDELINES.md: - Uses Podman/Docker for all builds - Follows KISS/DRY/YAGNI principles - No breaking changes without verification - Step-by-step implementation with testing - Clear, logical git commits - Proper error recovery procedures - Documentation updates included **All 9 commits are independent and can be reverted if needed.** **Proceed with implementation?**