From 9990a0d1ba5a960d74db1c2e5553cbdd735308da Mon Sep 17 00:00:00 2001 From: Adron Hall Date: Wed, 2 Sep 2026 09:26:55 -0700 Subject: [PATCH] fix(documents): send relativePath on offline sync ops so edits persist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Document edits and creates route through the offline sync path (default on) and are POSTed to /api/documents/sync. The backend gates every document create/update op on a required, folder-unique `relativePath` (`if (!id || !relativePath) continue;`) yet always returns 200, so the client silently believed the push succeeded: it cleared the outbox and marked the doc synced while the server dropped the op. The edit lived only in the optimistic cache until the next full pull merged the untouched server row back over it — so an edited doc reverted to its created-date state and never reached the server. The client `SyncOpData` already had a `relativePath` field; it was just never populated. Populate it at all three document op sites: - Document: decode `relativePath` from the sync response so pulled docs carry their real server path. - AppDataStore.updateDocumentOffline: echo the existing doc's path, falling back to ".md" for rows cached before the field existed. - AppDataStore.createDocumentOffline: generate a collision-safe ".md" (UUID basename can't violate @@unique([folderId, relativePath])). - DocumentSyncConflict.makeConflictCopy: give conflict copies a path too — they would otherwise fail to push for the same reason. Add tests covering the pushed create/update/conflict ops and the new model field, and log the backend defect (should not require relativePath on update; should fall back on create; 200-on-drop blind spot) as ask A7 in the-gaps.md. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01ATYNi3UKZpQH28AH6tCTup --- InterlinedList/Models/Document.swift | 8 +++- InterlinedList/Services/AppDataStore.swift | 20 +++++++--- .../Services/DocumentSyncConflict.swift | 8 +++- .../ModelTests/DocumentModelTests.swift | 9 +++++ .../ServiceTests/AppDataStoreTests.swift | 37 +++++++++++++++++++ .../DocumentSyncConflictTests.swift | 9 +++++ the-gaps.md | 15 ++++++++ 7 files changed, 99 insertions(+), 7 deletions(-) diff --git a/InterlinedList/Models/Document.swift b/InterlinedList/Models/Document.swift index 0aa21d2..6248026 100644 --- a/InterlinedList/Models/Document.swift +++ b/InterlinedList/Models/Document.swift @@ -13,13 +13,18 @@ struct Document: Codable, Identifiable, Hashable { let isPublic: Bool? let createdAt: String? let updatedAt: String? + /// Server file-path key (`@@unique([folderId, relativePath])`). Present on + /// `GET /api/documents/sync` rows; nil on other endpoints. The offline sync + /// `POST` requires a non-empty `relativePath` on create/update ops or it + /// silently drops them, so it's echoed back when queuing an edit. + let relativePath: String? /// Soft-delete tombstone from `GET /api/documents/sync`. `nil` on every other /// endpoint. A non-nil value means the row was deleted server-side. let deletedAt: String? init(id: String, title: String, content: String? = nil, folderId: String? = nil, isPublic: Bool? = nil, createdAt: String? = nil, updatedAt: String? = nil, - deletedAt: String? = nil) { + relativePath: String? = nil, deletedAt: String? = nil) { self.id = id self.title = title self.content = content @@ -27,6 +32,7 @@ struct Document: Codable, Identifiable, Hashable { self.isPublic = isPublic self.createdAt = createdAt self.updatedAt = updatedAt + self.relativePath = relativePath self.deletedAt = deletedAt } } diff --git a/InterlinedList/Services/AppDataStore.swift b/InterlinedList/Services/AppDataStore.swift index 7332a81..b6f24f8 100644 --- a/InterlinedList/Services/AppDataStore.swift +++ b/InterlinedList/Services/AppDataStore.swift @@ -350,13 +350,18 @@ final class AppDataStore: ObservableObject { func createDocumentOffline(title: String, content: String?, isPublic: Bool, folderId: String?) -> Document { let normalizedFolderId = (folderId?.isEmpty == true) ? nil : folderId let now = ISO8601DateFormatter().string(from: Date()) - let doc = Document(id: UUID().uuidString, title: title, content: content, + let id = UUID().uuidString + // The sync POST requires a non-empty, folder-unique relativePath; a fresh + // UUID basename can't collide with the `@@unique([folderId, relativePath])`. + let relativePath = "\(id).md" + let doc = Document(id: id, title: title, content: content, folderId: normalizedFolderId, isPublic: isPublic, - createdAt: now, updatedAt: now) + createdAt: now, updatedAt: now, relativePath: relativePath) documents.insert(doc, at: 0) enqueue(SyncOperation(op: .create, type: .document, data: SyncOpData(id: doc.id, folderId: normalizedFolderId, - title: title, content: content, isPublic: isPublic))) + title: title, content: content, + relativePath: relativePath, isPublic: isPublic))) return doc } @@ -365,9 +370,13 @@ final class AppDataStore: ObservableObject { let normalizedFolderId = (folderId?.isEmpty == true) ? nil : folderId let now = ISO8601DateFormatter().string(from: Date()) let existing = documents.first { $0.id == id } + // Echo the doc's existing path; fall back for rows cached before the field + // existed. Without a non-empty relativePath the server drops the update op. + let relativePath = existing?.relativePath ?? "\(id).md" let updated = Document(id: id, title: title, content: content, folderId: normalizedFolderId, isPublic: isPublic, - createdAt: existing?.createdAt, updatedAt: now) + createdAt: existing?.createdAt, updatedAt: now, + relativePath: relativePath) if let idx = documents.firstIndex(where: { $0.id == id }) { documents[idx] = updated } else { @@ -375,7 +384,8 @@ final class AppDataStore: ObservableObject { } enqueue(SyncOperation(op: .update, type: .document, data: SyncOpData(id: id, folderId: normalizedFolderId, - title: title, content: content, isPublic: isPublic))) + title: title, content: content, + relativePath: relativePath, isPublic: isPublic))) return updated } diff --git a/InterlinedList/Services/DocumentSyncConflict.swift b/InterlinedList/Services/DocumentSyncConflict.swift index b0c62c7..8b027fc 100644 --- a/InterlinedList/Services/DocumentSyncConflict.swift +++ b/InterlinedList/Services/DocumentSyncConflict.swift @@ -56,18 +56,24 @@ enum DocumentSyncConflict { static func makeConflictCopy(server: Document, date: Date, newId: String) -> ConflictCopy { let title = conflictCopyTitle(original: server.title, date: date) let now = ISO8601DateFormatter().string(from: date) + // A fresh basename for the copy; the sync POST drops a create op without a + // non-empty relativePath, and reusing the server doc's path would collide + // with `@@unique([folderId, relativePath])`. + let relativePath = "\(newId).md" let document = Document(id: newId, title: title, content: server.content, folderId: server.folderId, isPublic: server.isPublic, createdAt: now, - updatedAt: now) + updatedAt: now, + relativePath: relativePath) let operation = SyncOperation(op: .create, type: .document, data: SyncOpData(id: newId, folderId: server.folderId, title: title, content: server.content, + relativePath: relativePath, isPublic: server.isPublic)) return ConflictCopy(document: document, operation: operation) } diff --git a/InterlinedListTests/ModelTests/DocumentModelTests.swift b/InterlinedListTests/ModelTests/DocumentModelTests.swift index ff91113..4f14971 100644 --- a/InterlinedListTests/ModelTests/DocumentModelTests.swift +++ b/InterlinedListTests/ModelTests/DocumentModelTests.swift @@ -27,6 +27,15 @@ final class DocumentCodableTests: XCTestCase { XCTAssertNil(d.content) XCTAssertNil(d.folderId) XCTAssertNil(d.isPublic) + XCTAssertNil(d.relativePath) + } + + func test_decode_relativePathFromSyncRow() throws { + // `GET /api/documents/sync` rows carry the server file-path key that the + // client echoes back on offline edits. + let json = #"{"id":"d3","title":"Notes","relative_path":"notes/notes.md"}"# + let d = try decoder.decode(Document.self, from: Data(json.utf8)) + XCTAssertEqual(d.relativePath, "notes/notes.md") } func test_decode_documentsResponse() throws { diff --git a/InterlinedListTests/ServiceTests/AppDataStoreTests.swift b/InterlinedListTests/ServiceTests/AppDataStoreTests.swift index 4f0c99b..491bfb3 100644 --- a/InterlinedListTests/ServiceTests/AppDataStoreTests.swift +++ b/InterlinedListTests/ServiceTests/AppDataStoreTests.swift @@ -122,6 +122,43 @@ final class AppDataStoreTests: XCTestCase { XCTAssertTrue(store.pendingSyncDocIds.contains(doc.id)) } + func test_createDocumentOffline_pushedOpCarriesRelativePath() async { + // The sync POST silently drops a create/update op without a non-empty + // relativePath, so the queued op (and the optimistic doc) must carry one. + let api = RecordingSyncAPI(pushCursor: "c1", + pullResponse: DocumentSyncResponse(lastSyncAt: "c1")) + let store = AppDataStore(syncAPI: api) + let doc = store.createDocumentOffline(title: "Draft", content: "hi", isPublic: false, folderId: nil) + await store.pushOutbox() + let op = api.pushedOperations.first { $0.data.id == doc.id } + XCTAssertEqual(op?.op, .create) + XCTAssertEqual(op?.data.relativePath?.isEmpty, false) + XCTAssertEqual(store.documents.first { $0.id == doc.id }?.relativePath?.isEmpty, false) + } + + func test_updateDocumentOffline_pushedOpEchoesExistingRelativePath() async { + // Seed a pulled doc with a known server path, then edit it: the update op + // must echo that path so the server accepts the edit. + let api = ControllableSyncAPI() + api.nextPull = DocumentSyncResponse( + documents: [Document(id: "d1", title: "Server", content: "v1", folderId: nil, + isPublic: false, createdAt: "2026-07-01T00:00:00Z", + updatedAt: "2026-07-01T00:00:00Z", relativePath: "server-path.md")], + lastSyncAt: "c1") + let store = AppDataStore(syncAPI: api) + await store.pushOutbox() + + api.nextPull = nil + _ = store.updateDocumentOffline(id: "d1", title: "Edited", content: "v2", + isPublic: false, folderId: nil) + api.nextPull = DocumentSyncResponse(lastSyncAt: "c2") + await store.pushOutbox() + + let op = api.lastPushedOps.first { $0.data.id == "d1" } + XCTAssertEqual(op?.op, .update) + XCTAssertEqual(op?.data.relativePath, "server-path.md") + } + func test_pushOutbox_failure_keepsPendingState() async { let api = FailingSyncAPI() let store = AppDataStore(syncAPI: api) diff --git a/InterlinedListTests/ServiceTests/DocumentSyncConflictTests.swift b/InterlinedListTests/ServiceTests/DocumentSyncConflictTests.swift index 90aa929..78e0752 100644 --- a/InterlinedListTests/ServiceTests/DocumentSyncConflictTests.swift +++ b/InterlinedListTests/ServiceTests/DocumentSyncConflictTests.swift @@ -125,6 +125,15 @@ final class DocumentSyncConflictTests: XCTestCase { XCTAssertEqual(copy.operation.data.isPublic, false) } + func test_makeConflictCopy_opCarriesNonEmptyRelativePath() { + // The sync POST silently drops a create op without a relativePath, so the + // copy must carry one (and the copy document mirrors it). + let server = doc("d1", title: "T", updatedAt: "2026-08-02T00:00:00Z") + let copy = DocumentSyncConflict.makeConflictCopy(server: server, date: fixedDate, newId: "n1") + XCTAssertEqual(copy.operation.data.relativePath?.isEmpty, false) + XCTAssertEqual(copy.document.relativePath, copy.operation.data.relativePath) + } + func test_conflictCopyTitle_isDeterministicForDate() { XCTAssertEqual( DocumentSyncConflict.conflictCopyTitle(original: "X", date: fixedDate), diff --git a/the-gaps.md b/the-gaps.md index 7c93a54..5f34aa9 100644 --- a/the-gaps.md +++ b/the-gaps.md @@ -645,6 +645,21 @@ field). What's left: - **A6 — Doc the new endpoints (low priority).** `/api/tags/{trending,autocomplete}` (G13) and `/api/link-metadata` (G14) shipped but aren't yet in `/help/api/*`; add pages so future integrators (and this doc) can rely on the published contract. +- **A7 — `POST /api/documents/sync` silently drops document create/update ops that + omit `relativePath`.** The handler gates *both* create and update on + `if (!id || !relativePath) continue;` (`app/api/documents/sync/route.ts:194`), so + an op without a `relativePath` is skipped — yet the response is always + `200 { lastSyncAt }`, so the client can't tell the op was dropped. Two defects: + (1) **update shouldn't require `relativePath`** — the row already exists and the + update branch never writes the field, it's only used as a gate; require just `id`. + (2) **create should fall back** the way `POST /api/documents` does + (`route.ts:61`, `relativePath ?? \`${title-slug}.md\``) instead of dropping the row. + Ideally the POST response should also echo per-op results (applied/skipped) so + clients can reconcile instead of assuming success. **iOS worked around this + 2026-09-02** by generating/echoing a `relativePath` on every sync create/update + and conflict-copy op (see `AppDataStore.{create,update}DocumentOffline`, + `DocumentSyncConflict.makeConflictCopy`); the server-side hardening above is still + worth doing for other clients (`il-sync`, web) and for the silent-failure blind spot. ---