From ec4bcc75176302feed98b09b4aedbfac7c5550aa Mon Sep 17 00:00:00 2001 From: Sidney von Katzendame Date: Sat, 23 May 2026 20:17:37 -0400 Subject: [PATCH] fix(extension-chrome): propagate remote title/URL edits to local tree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously applyRemoteChanges early-returned when a remote bookmark was already in the local id map ('assume in sync; next reconcile fixes drift'). A remote title or URL edit on Device B was invisible on Device A for up to an hour, until the periodic reconcile re-walked both sides. When a remote bookmark already has a mapping, fetch the current local node via chrome.bookmarks.get, compare title/url, and call chrome.bookmarks.update with only the changed fields. Suppress both the old URL (in case it's being changed away) and the new URL so the resulting onChanged echo doesn't bounce back to GitHub. The 5-minute poll cycle now propagates edits in seconds, not hours. TDD: 3 new tests in apply-remote.test.ts (title change → update, URL change → update, no-op when matching → no update). Watched 2/3 fail RED (skip-no-op already passed because original code never called update). Implementation added a new applyRemoteEdit helper; all 12 apply-remote tests + full suite (72) pass GREEN. Closes #1 --- .../extension-chrome/src/lib/apply-remote.ts | 33 +++++++++- .../test/apply-remote.test.ts | 63 +++++++++++++++++++ packages/extension-chrome/test/setup.ts | 1 + 3 files changed, 96 insertions(+), 1 deletion(-) diff --git a/packages/extension-chrome/src/lib/apply-remote.ts b/packages/extension-chrome/src/lib/apply-remote.ts index 9a04a6a..df01396 100644 --- a/packages/extension-chrome/src/lib/apply-remote.ts +++ b/packages/extension-chrome/src/lib/apply-remote.ts @@ -38,7 +38,10 @@ export async function applyRemoteChanges( } if (existingNode != null) { - // Already in the local tree — assume in sync; next reconcile fixes drift. + // Remote bookmark already in the local tree — propagate title/url + // changes so users on Device A see edits from Device B within the + // 5-minute poll window (issue #1). + await applyRemoteEdit(existingNode, bm.url, bm.title); continue; } @@ -63,6 +66,34 @@ export async function applyRemoteChanges( } } +async function applyRemoteEdit( + nodeId: string, + remoteUrl: string, + remoteTitle: string, +): Promise { + let current: chrome.bookmarks.BookmarkTreeNode | undefined; + try { + const found = await chrome.bookmarks.get(nodeId); + current = found[0]; + } catch { + // Node may have been deleted locally between mapping and apply; skip. + return; + } + if (current == null) return; + + const changes: { title?: string; url?: string } = {}; + if (current.title !== remoteTitle) changes.title = remoteTitle; + if (current.url !== remoteUrl) changes.url = remoteUrl; + if (Object.keys(changes).length === 0) return; + + // Suppress both the old URL (in case it's being changed away) and the new + // URL — the resulting onChanged echo will report the new URL. + if (current.url != null && current.url.length > 0) suppress(current.url); + suppress(remoteUrl); + + await chrome.bookmarks.update(nodeId, changes); +} + async function ensureFolderPath( folder: string, bookmarksBarId: string, diff --git a/packages/extension-chrome/test/apply-remote.test.ts b/packages/extension-chrome/test/apply-remote.test.ts index ce114a7..efb2cb5 100644 --- a/packages/extension-chrome/test/apply-remote.test.ts +++ b/packages/extension-chrome/test/apply-remote.test.ts @@ -44,6 +44,69 @@ describe("applyRemoteChanges", () => { expect(isSuppressed("https://example.com/new")).toBe(true); }); + it("propagates a remote title change to the local node (issue #1)", async () => { + const bm = bookmark({ + id: "u1", + url: "https://example.com/", + title: "New title from another device", + }); + const idMap = await IdMap.load(); + idMap.set(asUlid("u1"), asNodeId("node-1")); + + // Current local node has the old title + (chrome.bookmarks.get as any).mockResolvedValueOnce([ + { id: "node-1", parentId: BAR, title: "Old title", url: "https://example.com/" }, + ]); + + await applyRemoteChanges(file([bm]), idMap, BAR, OTHER); + + expect(chrome.bookmarks.update).toHaveBeenCalledWith("node-1", { + title: "New title from another device", + }); + // The URL is unchanged, but we still suppress to avoid an onChanged echo + expect(isSuppressed("https://example.com/")).toBe(true); + }); + + it("propagates a remote URL change to the local node (issue #1)", async () => { + const bm = bookmark({ + id: "u1", + url: "https://example.com/new-path", + title: "Same title", + }); + const idMap = await IdMap.load(); + idMap.set(asUlid("u1"), asNodeId("node-1")); + + (chrome.bookmarks.get as any).mockResolvedValueOnce([ + { id: "node-1", parentId: BAR, title: "Same title", url: "https://example.com/old-path" }, + ]); + + await applyRemoteChanges(file([bm]), idMap, BAR, OTHER); + + expect(chrome.bookmarks.update).toHaveBeenCalledWith("node-1", { + url: "https://example.com/new-path", + }); + // Suppress both old and new URL — the onChanged echo will fire with the new URL + expect(isSuppressed("https://example.com/new-path")).toBe(true); + }); + + it("skips the update call when remote matches local (no spurious onChanged)", async () => { + const bm = bookmark({ + id: "u1", + url: "https://example.com/", + title: "Same", + }); + const idMap = await IdMap.load(); + idMap.set(asUlid("u1"), asNodeId("node-1")); + + (chrome.bookmarks.get as any).mockResolvedValueOnce([ + { id: "node-1", parentId: BAR, title: "Same", url: "https://example.com/" }, + ]); + + await applyRemoteChanges(file([bm]), idMap, BAR, OTHER); + + expect(chrome.bookmarks.update).not.toHaveBeenCalled(); + }); + it("does not create a bookmark already mapped", async () => { const bm = bookmark({ id: "u1", url: "https://example.com/" }); const idMap = await IdMap.load(); diff --git a/packages/extension-chrome/test/setup.ts b/packages/extension-chrome/test/setup.ts index 6d9dcf0..244362e 100644 --- a/packages/extension-chrome/test/setup.ts +++ b/packages/extension-chrome/test/setup.ts @@ -43,6 +43,7 @@ const chromeStub = { update: vi.fn(async () => ({} as chrome.bookmarks.BookmarkTreeNode)), move: vi.fn(async () => ({} as chrome.bookmarks.BookmarkTreeNode)), remove: vi.fn(async () => {}), + get: vi.fn(async () => [] as chrome.bookmarks.BookmarkTreeNode[]), getTree: vi.fn(async () => [] as chrome.bookmarks.BookmarkTreeNode[]), getSubTree: vi.fn(async () => [] as chrome.bookmarks.BookmarkTreeNode[]), onCreated: { addListener: vi.fn(), removeListener: vi.fn() },