diff --git a/apps/desktop/package.json b/apps/desktop/package.json index 75703951..9cfe4b27 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -1,7 +1,7 @@ { "name": "@zennotes/desktop", "productName": "ZenNotes", - "version": "2.46.0", + "version": "2.47.0", "description": "ZenNotes desktop shell", "private": true, "main": "./out/main/index.js", diff --git a/apps/desktop/src/main/cloud-sync-service.test.ts b/apps/desktop/src/main/cloud-sync-service.test.ts index d762bc21..f4cfc47f 100644 --- a/apps/desktop/src/main/cloud-sync-service.test.ts +++ b/apps/desktop/src/main/cloud-sync-service.test.ts @@ -10,7 +10,7 @@ import type { } from '@zennotes/bridge-contract/cloud-sync' import { afterEach, describe, expect, it, vi } from 'vitest' import { CloudServiceRequestError } from './cloud-sync-client' -import { DesktopCloudSyncService } from './cloud-sync-service' +import { DesktopCloudSyncService, type DesktopCloudSyncServiceDependencies } from './cloud-sync-service' const temporaryDirectories: string[] = [] @@ -25,7 +25,8 @@ afterEach(async () => { async function setup( vaults: CloudSyncVault[] = [], fetchImplementation?: typeof fetch, - accountStatus?: () => Promise + accountStatus?: () => Promise, + withWindowSync?: DesktopCloudSyncServiceDependencies['withWindowSync'] ) { const localRoot = await mkdtemp(path.join(os.tmpdir(), 'zennotes-local-vault-')) const storageDirectory = await mkdtemp(path.join(os.tmpdir(), 'zennotes-cloud-state-')) @@ -189,12 +190,43 @@ async function setup( getSecret: async () => 'secret-token', createClient: () => client, fetchImplementation, + withWindowSync, now: () => new Date('2026-08-10T12:00:00.000Z') }) return { service, client, localRoot } } describe('DesktopCloudSyncService', () => { + it('prepares windows before taking the vault lock and coalesces preparation', async () => { + let prepared!: () => void + const gate = new Promise((resolve) => { prepared = resolve }) + let prepareWork = async (): Promise => {} + const prepare = vi.fn>(async (_root, run) => { + await gate + await prepareWork() + return await run() + }) + const vault: CloudSyncVault = { + id: 'vault-1', name: 'Notes', cursor: 0, + created_at: '2026-08-10T12:00:00.000Z', updated_at: '2026-08-10T12:00:00.000Z' + } + const { service, client, localRoot } = await setup([vault], undefined, undefined, prepare) + await service.link(localRoot, vault.id) + // getConflict uses the same exclusive queue as saveConflictDraft. If the + // sync takes that lock first, this awaited read would deadlock. + prepareWork = async () => { + await expect(service.getConflict(localRoot, 'not-pending')).rejects.toThrow() + } + const first = service.sync(localRoot) + const second = service.sync(localRoot) + expect(first).toBe(second) + expect(prepare).toHaveBeenCalledOnce() + expect(client.manifest).not.toHaveBeenCalled() + prepared() + await Promise.all([first, second]) + expect(client.manifest).toHaveBeenCalledOnce() + }) + // Settings differ between devices, so sync asks instead of picking. Doing // nothing keeps this device's settings, which are already in use. it('answers the settings question either way', async () => { diff --git a/apps/desktop/src/main/cloud-sync-service.ts b/apps/desktop/src/main/cloud-sync-service.ts index fe6a2927..f90dfaf5 100644 --- a/apps/desktop/src/main/cloud-sync-service.ts +++ b/apps/desktop/src/main/cloud-sync-service.ts @@ -66,6 +66,7 @@ export interface DesktopCloudSyncServiceDependencies { createClient(baseUrl: string, token: string): SyncClient fetchImplementation?: typeof fetch now?: () => Date + withWindowSync?(root: string, run: () => Promise): Promise } /** Main-process orchestration for linking one local vault to one cloud vault. */ @@ -295,7 +296,8 @@ export class DesktopCloudSyncService { const existing = this.runs.get(runKey) if (existing) return existing - const running = this.exclusive(runKey, () => this.run(localRoot)).finally(() => { + const run = () => this.exclusive(runKey, () => this.run(localRoot)) + const running = (this.dependencies.withWindowSync?.(runKey, run) ?? run()).finally(() => { this.runs.delete(runKey) }) this.runs.set(runKey, running) diff --git a/apps/desktop/src/main/cloud-sync-window-barrier.test.ts b/apps/desktop/src/main/cloud-sync-window-barrier.test.ts new file mode 100644 index 00000000..5034973e --- /dev/null +++ b/apps/desktop/src/main/cloud-sync-window-barrier.test.ts @@ -0,0 +1,356 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { + CloudSyncRunSummary, + CloudSyncWindowEvent +} from '@zennotes/bridge-contract/cloud-sync' +import { CloudSyncWindowBarrier } from './cloud-sync-window-barrier' + +const root = '/test/vault' +const summary: CloudSyncRunSummary = { + cursor: 3, + pulled: 1, + pushed: 0, + conflicts: [], + bootstrap_conflicts: [], + local_conflicts: [], + pending_conflicts: [] +} + +function windowParticipant(id: number) { + const events: CloudSyncWindowEvent[] = [] + return { id, events, send: (event: CloudSyncWindowEvent) => events.push(event) } +} + +function requestId(participant: ReturnType): string { + const prepare = participant.events.find((event) => event.phase === 'prepare') + expect(prepare).toBeDefined() + return prepare!.requestId +} + +beforeEach(() => vi.useFakeTimers()) +afterEach(() => vi.useRealTimers()) + +describe('CloudSyncWindowBarrier', () => { + it('allows only one review writer per vault/conflict, including same-window mounts', () => { + const barrier = new CloudSyncWindowBarrier({ participants: () => [] }) + barrier.claimReview(1, root, 'note', 'review-a') + expect(() => barrier.claimReview(1, root, 'note', 'review-a')).not.toThrow() + expect(() => barrier.claimReview(2, root, 'note', 'review-b')).toThrow( + 'another ZenNotes window' + ) + expect(() => barrier.claimReview(1, root, 'note', 'review-b')).toThrow( + 'another ZenNotes window' + ) + expect(() => barrier.claimReview(2, root, 'other', 'review-b')).not.toThrow() + expect(() => barrier.claimReview(2, '/other/vault', 'note', 'review-b')).not.toThrow() + barrier.releaseReview(2, 'note', 'review-a') + barrier.releaseReview(1, 'note', 'stale-session') + expect(() => barrier.claimReview(2, root, 'note', 'review-b')).toThrow() + barrier.releaseReview(1, 'note', 'review-a') + expect(() => barrier.claimReview(2, root, 'note', 'review-b')).not.toThrow() + barrier.releaseReview(2) + expect(() => barrier.claimReview(1, root, 'note', 'review-c')).not.toThrow() + }) + + it('prepares every window and waits for every draft acknowledgement before syncing', async () => { + const first = windowParticipant(1) + const second = windowParticipant(2) + const barrier = new CloudSyncWindowBarrier({ participants: () => [first, second] }) + const work = vi.fn(async () => summary) + + const result = barrier.run(root, work) + await vi.advanceTimersByTimeAsync(0) + const id = requestId(first) + expect(second.events).toEqual([{ phase: 'prepare', requestId: id }]) + expect(work).not.toHaveBeenCalled() + + barrier.acknowledge(first.id, id, null) + await vi.advanceTimersByTimeAsync(0) + expect(work).not.toHaveBeenCalled() + barrier.acknowledge(second.id, id, null) + + await expect(result).resolves.toEqual(summary) + expect(work).toHaveBeenCalledTimes(1) + for (const participant of [first, second]) { + expect(participant.events).toEqual([ + { phase: 'prepare', requestId: id }, + { phase: 'finished', requestId: id, summary, error: null } + ]) + } + }) + + it('ignores unrelated senders, stale requests, and duplicate acknowledgements', async () => { + const first = windowParticipant(1) + const second = windowParticipant(2) + const barrier = new CloudSyncWindowBarrier({ participants: () => [first, second] }) + const work = vi.fn(async () => summary) + const result = barrier.run(root, work) + await vi.advanceTimersByTimeAsync(0) + const id = requestId(first) + + barrier.acknowledge(99, id, null) + barrier.acknowledge(second.id, 'previous-request', null) + barrier.acknowledge(99, id, 'Unrelated error') + barrier.acknowledge(first.id, id, null) + barrier.acknowledge(first.id, id, null) + await vi.advanceTimersByTimeAsync(0) + expect(work).not.toHaveBeenCalled() + + barrier.acknowledge(second.id, id, null) + await expect(result).resolves.toEqual(summary) + expect(work).toHaveBeenCalledTimes(1) + }) + + it('aborts and releases all prepared windows when a draft cannot be saved', async () => { + const first = windowParticipant(1) + const second = windowParticipant(2) + const barrier = new CloudSyncWindowBarrier({ participants: () => [first, second] }) + const work = vi.fn(async () => summary) + const outcome = barrier.run(root, work).catch((error: unknown) => error) + await vi.advanceTimersByTimeAsync(0) + const id = requestId(first) + + barrier.acknowledge(first.id, id, 'Cannot save conflict draft') + expect(await outcome).toBeInstanceOf(Error) + expect(work).not.toHaveBeenCalled() + for (const participant of [first, second]) { + expect(participant.events.at(-1)).toEqual({ + phase: 'finished', + requestId: id, + summary: null, + error: expect.stringContaining('Cannot save conflict draft') + }) + } + }) + + it('times out without running sync and does not accept a late acknowledgement', async () => { + const participant = windowParticipant(1) + const barrier = new CloudSyncWindowBarrier({ + participants: () => [participant], + timeoutMs: 100 + }) + const work = vi.fn(async () => summary) + const outcome = barrier.run(root, work).catch((error: unknown) => error) + await vi.advanceTimersByTimeAsync(0) + const id = requestId(participant) + + await vi.advanceTimersByTimeAsync(100) + expect(await outcome).toBeInstanceOf(Error) + barrier.acknowledge(participant.id, id, null) + await vi.advanceTimersByTimeAsync(0) + expect(work).not.toHaveBeenCalled() + expect(participant.events).toEqual([ + { phase: 'prepare', requestId: id }, + { phase: 'finished', requestId: id, summary: null, error: expect.any(String) } + ]) + }) + + it.each(['opened', 'closed'] as const)( + 'aborts if another window is %s before all drafts are saved', + async (change) => { + const first = windowParticipant(1) + const second = windowParticipant(2) + let participants = [first, second] + const barrier = new CloudSyncWindowBarrier({ participants: () => participants }) + const work = vi.fn(async () => summary) + const outcome = barrier.run(root, work).catch((error: unknown) => error) + await vi.advanceTimersByTimeAsync(0) + const id = requestId(first) + barrier.acknowledge(first.id, id, null) + participants = change === 'opened' ? [...participants, windowParticipant(3)] : [first] + barrier.acknowledge(second.id, id, null) + + expect(await outcome).toBeInstanceOf(Error) + expect(work).not.toHaveBeenCalled() + expect(first.events.at(-1)).toEqual({ + phase: 'finished', + requestId: id, + summary: null, + error: expect.any(String) + }) + } + ) + + it('releases prepared windows when the actual sync fails', async () => { + const participant = windowParticipant(1) + const barrier = new CloudSyncWindowBarrier({ participants: () => [participant] }) + const failure = new Error('Cloud connection failed') + const work = vi.fn(async () => { + throw failure + }) + const outcome = barrier.run(root, work).catch((error: unknown) => error) + await vi.advanceTimersByTimeAsync(0) + const id = requestId(participant) + barrier.acknowledge(participant.id, id, null) + + expect(await outcome).toBe(failure) + expect(participant.events.at(-1)).toEqual({ + phase: 'finished', + requestId: id, + summary: null, + error: 'Cloud connection failed' + }) + }) + + it('runs immediately when no window is attached to the vault', async () => { + const work = vi.fn(async () => summary) + const barrier = new CloudSyncWindowBarrier({ participants: () => [] }) + await expect(barrier.run(root, work)).resolves.toEqual(summary) + expect(work).toHaveBeenCalledTimes(1) + }) + + it('aborts if preparing a window throws and still releases other prepared windows', async () => { + const first = windowParticipant(1) + const second = windowParticipant(2) + const failure = new Error('Window closed before it could save') + const barrier = new CloudSyncWindowBarrier({ + participants: () => [ + first, + { + ...second, + send(event) { + second.send(event) + throw failure + } + } + ] + }) + const work = vi.fn(async () => summary) + + await expect(barrier.run(root, work)).rejects.toBe(failure) + + expect(work).not.toHaveBeenCalled() + expect(first.events.at(-1)).toEqual({ + phase: 'finished', + requestId: requestId(first), + summary: null, + error: failure.message + }) + expect(vi.getTimerCount()).toBe(0) + }) + + it('does not let one failed completion notification strand another window', async () => { + const first = windowParticipant(1) + const second = windowParticipant(2) + const barrier = new CloudSyncWindowBarrier({ + participants: () => [ + { + ...first, + send(event) { + first.send(event) + if (event.phase === 'finished') throw new Error('Window closed') + } + }, + second + ] + }) + const result = barrier.run(root, async () => summary) + await vi.advanceTimersByTimeAsync(0) + const id = requestId(first) + barrier.acknowledge(first.id, id, null) + barrier.acknowledge(second.id, id, null) + + await expect(result).resolves.toEqual(summary) + expect(second.events.at(-1)).toEqual({ + phase: 'finished', + requestId: id, + summary, + error: null + }) + }) + + it('requires a fresh acknowledgement when retrying a timed-out sync', async () => { + const participant = windowParticipant(1) + const barrier = new CloudSyncWindowBarrier({ + participants: () => [participant], + timeoutMs: 100 + }) + const work = vi.fn(async () => summary) + const failed = barrier.run(root, work).catch((error: unknown) => error) + await vi.advanceTimersByTimeAsync(0) + const oldId = requestId(participant) + await vi.advanceTimersByTimeAsync(100) + expect(await failed).toBeInstanceOf(Error) + participant.events.length = 0 + + const retried = barrier.run(root, work) + await vi.advanceTimersByTimeAsync(0) + const freshId = requestId(participant) + expect(freshId).not.toBe(oldId) + barrier.acknowledge(participant.id, oldId, null) + await vi.advanceTimersByTimeAsync(0) + expect(work).not.toHaveBeenCalled() + barrier.acknowledge(participant.id, freshId, null) + + await expect(retried).resolves.toEqual(summary) + expect(work).toHaveBeenCalledTimes(1) + expect(vi.getTimerCount()).toBe(0) + }) + + it('supports immediate acknowledgements and does not apply the preparation timeout to sync work', async () => { + const first = windowParticipant(1) + const second = windowParticipant(2) + let finish!: (value: CloudSyncRunSummary) => void + const work = vi.fn( + () => + new Promise((resolve) => { + finish = resolve + }) + ) + const barrier = new CloudSyncWindowBarrier({ + participants: () => + [first, second].map((participant) => ({ + id: participant.id, + send(event) { + participant.send(event) + if (event.phase === 'prepare') + barrier.acknowledge(participant.id, event.requestId, null) + } + })), + timeoutMs: 100 + }) + + const result = barrier.run(root, work) + await vi.advanceTimersByTimeAsync(500) + expect(work).toHaveBeenCalledTimes(1) + expect(first.events).toHaveLength(1) + expect(second.events).toHaveLength(1) + finish(summary) + await expect(result).resolves.toEqual(summary) + expect(vi.getTimerCount()).toBe(0) + }) + + it('keeps draft acknowledgements isolated between concurrent vault syncs', async () => { + const first = windowParticipant(1) + const second = windowParticipant(2) + const otherRoot = '/test/other-vault' + const barrier = new CloudSyncWindowBarrier({ + participants: (vaultRoot) => (vaultRoot === root ? [first] : [second]) + }) + const firstWork = vi.fn(async () => summary) + const otherSummary = { ...summary, cursor: 9 } + const secondWork = vi.fn(async () => otherSummary) + const firstResult = barrier.run(root, firstWork) + const secondResult = barrier.run(otherRoot, secondWork) + await vi.advanceTimersByTimeAsync(0) + const firstId = requestId(first) + const secondId = requestId(second) + expect(firstId).not.toBe(secondId) + + barrier.acknowledge(first.id, secondId, null) + barrier.acknowledge(second.id, firstId, null) + barrier.acknowledge(first.id, firstId, null) + await expect(firstResult).resolves.toEqual(summary) + expect(secondWork).not.toHaveBeenCalled() + expect(second.events).toEqual([{ phase: 'prepare', requestId: secondId }]) + + barrier.acknowledge(second.id, secondId, null) + await expect(secondResult).resolves.toEqual(otherSummary) + expect(second.events.at(-1)).toEqual({ + phase: 'finished', + requestId: secondId, + summary: otherSummary, + error: null + }) + }) +}) diff --git a/apps/desktop/src/main/cloud-sync-window-barrier.ts b/apps/desktop/src/main/cloud-sync-window-barrier.ts new file mode 100644 index 00000000..a6e53634 --- /dev/null +++ b/apps/desktop/src/main/cloud-sync-window-barrier.ts @@ -0,0 +1,113 @@ +import { randomUUID } from 'node:crypto' +import type { + CloudSyncRunSummary, + CloudSyncWindowEvent +} from '@zennotes/bridge-contract/cloud-sync' + +interface Participant { + id: number + send(event: CloudSyncWindowEvent): void +} + +/** Flush outside the vault's exclusive lock: saving a draft needs that lock. */ +export class CloudSyncWindowBarrier { + private reviews = new Map() + private pending = new Map< + string, + { + waiting: Set + resolve(): void + reject(error: Error): void + } + >() + + constructor( + private readonly options: { + participants(root: string): Participant[] + timeoutMs?: number + } + ) {} + + claimReview(owner: number, root: string, conflictId: string, reviewId: string): void { + const key = JSON.stringify([root, conflictId]) + const current = this.reviews.get(key) + if (current && (current.owner !== owner || current.reviewId !== reviewId)) { + throw new Error( + 'This file is already being reviewed in another ZenNotes window. Finish that review first.' + ) + } + this.reviews.set(key, { owner, conflictId, reviewId }) + } + + releaseReview(owner: number, conflictId?: string, reviewId?: string): void { + for (const [key, review] of this.reviews) { + if ( + review.owner === owner && + (conflictId === undefined || review.conflictId === conflictId) && + (reviewId === undefined || review.reviewId === reviewId) + ) { + this.reviews.delete(key) + } + } + } + + acknowledge(senderId: number, requestId: string, error: string | null): void { + const pending = this.pending.get(requestId) + if (!pending?.waiting.delete(senderId)) return + if (error !== null) pending.reject(new Error(error)) + else if (pending.waiting.size === 0) pending.resolve() + } + + async run(root: string, work: () => Promise): Promise { + const participants = this.options.participants(root) + const requestId = randomUUID() + let timer: ReturnType | undefined + let summary: CloudSyncRunSummary | null = null + let error: string | null = null + try { + if (participants.length > 0) { + await new Promise((resolve, reject) => { + this.pending.set(requestId, { + waiting: new Set(participants.map((participant) => participant.id)), + resolve, + reject + }) + timer = setTimeout( + () => + reject( + new Error( + 'Another ZenNotes window could not save its review draft. Sync paused; try again.' + ) + ), + this.options.timeoutMs ?? 10_000 + ) + for (const participant of participants) participant.send({ phase: 'prepare', requestId }) + }) + } + clearTimeout(timer) + const current = this.options.participants(root) + if ( + current.length !== participants.length || + current.some( + (candidate) => !participants.some((participant) => participant.id === candidate.id) + ) + ) + throw new Error('Vault windows changed while preparing sync. Please try again.') + summary = await work() + return summary + } catch (cause) { + error = cause instanceof Error ? cause.message : String(cause) + throw cause + } finally { + clearTimeout(timer) + this.pending.delete(requestId) + for (const participant of participants) { + try { + participant.send({ phase: 'finished', requestId, summary, error }) + } catch { + /* A closed window must not prevent releasing the others. */ + } + } + } + } +} diff --git a/apps/desktop/src/main/index.ts b/apps/desktop/src/main/index.ts index 3eb65213..8787fd7d 100644 --- a/apps/desktop/src/main/index.ts +++ b/apps/desktop/src/main/index.ts @@ -193,6 +193,7 @@ import { shouldForceGnomeLibsecret } from "./linux-password-store"; import { CloudAuthLoopbackServer } from "./cloud-auth-loopback"; import { createCloudSyncClient } from "./cloud-sync-client"; import { DesktopCloudSyncService } from "./cloud-sync-service"; +import { CloudSyncWindowBarrier } from "./cloud-sync-window-barrier"; import { scanAllTasks, scanTasksForPath } from "./tasks"; import { readDatabase, @@ -428,6 +429,7 @@ function getCloudSyncService(): DesktopCloudSyncService { accountStatus: () => getCloudAuthManager().status(), getSecret: getCloudServiceSecret, createClient: createCloudSyncClient, + withWindowSync: (root, run) => cloudSyncWindowBarrier.run(root, run), }); return cloudSyncService; } @@ -665,6 +667,21 @@ function flushWindowNoteOpens(win: BrowserWindow): void { // panel looks like the app opened a quick note instead of the file. const workspaceWindowIds = new Set(); +const cloudSyncWindowBarrier = new CloudSyncWindowBarrier({ + participants: (root) => BrowserWindow.getAllWindows() + .filter((win) => !win.isDestroyed() && isWorkspaceWindow(win) && + windowVaults.vaultForWindow(win.id)?.root === root) + .map((win) => ({ + id: win.webContents.id, + send(event) { + if (win.isDestroyed() || windowVaults.vaultForWindow(win.id)?.root !== root) { + throw new Error("The vault window closed or changed vaults."); + } + win.webContents.send(IPC.CLOUD_VAULT_SYNC_WINDOW, event); + }, + })), +}); + function isWorkspaceWindow(win: BrowserWindow): boolean { return workspaceWindowIds.has(win.id); } @@ -2909,6 +2926,11 @@ function registerIpc(): void { handle(IPC.CLOUD_VAULT_SYNC, () => getCloudSyncService().sync(requireLocalCloudVaultRoot()), ); + on(IPC.CLOUD_VAULT_SYNC_WINDOW_ACK, (event, requestId: unknown, error: unknown) => { + if (typeof requestId !== "string" || requestId.length > 100 || + (error !== null && (typeof error !== "string" || error.length > 2000))) return; + cloudSyncWindowBarrier.acknowledge(event.sender.id, requestId, error); + }); handle( IPC.CLOUD_VAULT_BOOTSTRAP_CONFLICT_GET, (_event, conflict: CloudSyncBootstrapConflict) => @@ -2925,9 +2947,32 @@ function registerIpc(): void { resolution, ), ); - handle(IPC.CLOUD_VAULT_CONFLICT_GET, (_event, conflictId: string) => - getCloudSyncService().getConflict(requireLocalCloudVaultRoot(), conflictId), - ); + const reviewWindows = new Set(); + handle(IPC.CLOUD_VAULT_CONFLICT_GET, async (event, conflictId: string, reviewId = "legacy") => { + if (typeof conflictId !== "string" || conflictId.length > 200) throw new Error("Invalid conflict ID."); + if (typeof reviewId !== "string" || reviewId.length > 100) throw new Error("Invalid review ID."); + const owner = event.sender.id; + const root = requireLocalCloudVaultRoot(); + cloudSyncWindowBarrier.claimReview(owner, root, conflictId, reviewId); + if (!reviewWindows.has(owner)) { + reviewWindows.add(owner); + event.sender.once("destroyed", () => { + cloudSyncWindowBarrier.releaseReview(owner); + reviewWindows.delete(owner); + }); + } + try { + return await getCloudSyncService().getConflict(root, conflictId); + } catch (error) { + cloudSyncWindowBarrier.releaseReview(owner, conflictId, reviewId); + throw error; + } + }); + handle(IPC.CLOUD_VAULT_CONFLICT_REVIEW_RELEASE, (event, conflictId: unknown, reviewId: unknown) => { + if (typeof conflictId !== "string" || conflictId.length > 200) return; + if (typeof reviewId !== "string" || reviewId.length > 100) return; + cloudSyncWindowBarrier.releaseReview(event.sender.id, conflictId, reviewId); + }); handle( IPC.CLOUD_VAULT_CONFLICT_DRAFT_SAVE, (_event, conflictId: string, draftText: string | null) => diff --git a/apps/desktop/src/preload/index.ts b/apps/desktop/src/preload/index.ts index cf174741..bb943713 100644 --- a/apps/desktop/src/preload/index.ts +++ b/apps/desktop/src/preload/index.ts @@ -28,6 +28,8 @@ import type { CloudSyncPendingConflictDetails, CloudSyncPendingConflictResolution, CloudSyncRunSummary, + CloudSyncWindowEvent, + CloudSyncWindowHandlers, CloudSyncSettingsChoice, CloudSyncSettingsConflict, CloudSyncVault, @@ -256,6 +258,29 @@ const api: ZenBridge = { unlinkCloudVault: (): Promise => ipcRenderer.invoke(IPC.CLOUD_VAULT_LINK_DELETE), deleteCloudVault: (): Promise => ipcRenderer.invoke(IPC.CLOUD_VAULT_DELETE), syncCloudVault: (): Promise => ipcRenderer.invoke(IPC.CLOUD_VAULT_SYNC), + onCloudSyncWindow: (handlers: CloudSyncWindowHandlers): (() => void) => { + const active = new Set() + const listener = (_event: Electron.IpcRendererEvent, event: CloudSyncWindowEvent): void => { + if (event.phase === 'finished') { + if (active.delete(event.requestId)) handlers.finished(event.summary, event.error) + return + } + active.add(event.requestId) + void Promise.resolve().then(() => handlers.prepare()).then( + () => ipcRenderer.send(IPC.CLOUD_VAULT_SYNC_WINDOW_ACK, event.requestId, null), + () => ipcRenderer.send(IPC.CLOUD_VAULT_SYNC_WINDOW_ACK, event.requestId, + 'A review draft could not be saved in another window. Sync paused; try again.') + ) + } + ipcRenderer.on(IPC.CLOUD_VAULT_SYNC_WINDOW, listener) + return () => { + for (const requestId of active) { + ipcRenderer.send(IPC.CLOUD_VAULT_SYNC_WINDOW_ACK, requestId, 'The vault window changed during sync preparation.') + } + active.clear() + ipcRenderer.removeListener(IPC.CLOUD_VAULT_SYNC_WINDOW, listener) + } + }, getCloudBootstrapConflict: ( conflict: CloudSyncBootstrapConflict ): Promise => @@ -264,8 +289,10 @@ const api: ZenBridge = { resolution: CloudSyncBootstrapConflictResolution ): Promise => ipcRenderer.invoke(IPC.CLOUD_VAULT_BOOTSTRAP_CONFLICT_RESOLVE, resolution), - getCloudConflict: (conflictId: string): Promise => - ipcRenderer.invoke(IPC.CLOUD_VAULT_CONFLICT_GET, conflictId), + getCloudConflict: (conflictId: string, reviewId?: string): Promise => + ipcRenderer.invoke(IPC.CLOUD_VAULT_CONFLICT_GET, conflictId, reviewId), + releaseCloudConflictReview: (conflictId: string, reviewId: string): Promise => + ipcRenderer.invoke(IPC.CLOUD_VAULT_CONFLICT_REVIEW_RELEASE, conflictId, reviewId), saveCloudConflictDraft: (conflictId: string, draftText: string | null): Promise => ipcRenderer.invoke(IPC.CLOUD_VAULT_CONFLICT_DRAFT_SAVE, conflictId, draftText), resolveCloudConflict: (resolution: CloudSyncPendingConflictResolution): Promise => diff --git a/apps/server/package.json b/apps/server/package.json index e7b2caea..19ac9c4d 100644 --- a/apps/server/package.json +++ b/apps/server/package.json @@ -1,7 +1,7 @@ { "name": "@zennotes/server", "private": true, - "version": "2.46.0", + "version": "2.47.0", "scripts": { "dev": "node ../../tooling/scripts/run-go-server-dev.mjs", "prepare-web": "node ../../tooling/scripts/prepare-server-web-dist.mjs", diff --git a/apps/web/package.json b/apps/web/package.json index 50ae4206..4759e1fd 100644 --- a/apps/web/package.json +++ b/apps/web/package.json @@ -1,7 +1,7 @@ { "name": "@zennotes/web", "private": true, - "version": "2.46.0", + "version": "2.47.0", "type": "module", "description": "ZenNotes web client for self-hosted and hosted deployments", "homepage": "https://zennotes.org", diff --git a/docs/releases/v2.47.0/RELEASE_NOTES.md b/docs/releases/v2.47.0/RELEASE_NOTES.md new file mode 100644 index 00000000..957e4c46 --- /dev/null +++ b/docs/releases/v2.47.0/RELEASE_NOTES.md @@ -0,0 +1,35 @@ +ZenNotes 2.47.0: switching drawings no longer overwrites one with the other, wikilinks at files are files, tasks stay visible during Cloud conflicts, and matching files stop waiting for review + +> A data-loss fix for Excalidraw drawings from [#755](https://github.com/ZenNotes/zennotes/issues/755), the Connections panel and `gd` treating a wikilink at an image or PDF as a missing note from [#757](https://github.com/ZenNotes/zennotes/issues/757), and two fixes from Unyanda's September 8 report: a note waiting for a Cloud decision no longer takes its tasks out of your day, and copying the same complete file to both devices no longer leaves it stuck in review. Unfinished merge drafts stay protected, including across desktop windows. Release PR: [#752](https://github.com/ZenNotes/zennotes/pull/752). + +## 🐛 Fixes + +- **A wikilink at a file in the vault is a file, not a missing note** ([#757](https://github.com/ZenNotes/zennotes/issues/757), reported by Unyanda). The Connections panel listed an embedded image such as `![[assets/diagram.png]]` under Links From Here as an unresolved wikilink and offered to create `assets/diagram.png.md` for it. The panel only asked the note index whether a target resolved, and an image is not a note. It now checks the vault's files with the same rules that render the embed (relative to the note, relative to the vault root, then a unique file name), lists a match as a **file** row that opens the file in its own tab, and offers to create a note only for a wikilink that reaches nothing. The same step now runs in every place a wikilink is followed: `gd` in the editor, a click on a rendered wikilink, and Cmd/Ctrl-click, none of which should propose `file.png.md` either. PDFs keep their existing `gd` behaviour of pinning into the reference pane. + + How to test locally: launch the isolated desktop as described below, put a PNG in `assets/`, and write `![[assets/diagram.png]]` and `[[assets/diagram.png]]` in a note. Press **Mod+2**. Before: a `CREATE` row for `assets/diagram.png` suggesting `/assets/diagram.png.md`. After: a `FILE` row named `diagram.png`; `j` onto it and Enter opens the image in a tab. Put the cursor inside `[[assets/diagram.png]]` and press `gd` in normal mode. Before: a "Create note for" prompt. After: the image tab opens. + +- **Switching between drawings no longer overwrites the one you open** ([#755](https://github.com/ZenNotes/zennotes/issues/755), reported by gverger). Opening a second Excalidraw drawing could replace its file with the scene of the drawing you just left, and switching back and forth left both files holding the same scene. The drawing editor kept one "latest scene" and wrote it, after its short save delay, to whichever drawing was showing when the delay ran out. On a local vault the next drawing loads within a millisecond, so React folded "clear the canvas" and "show the next drawing" into one update and the previous canvas was never torn down. Excalidraw only reads the scene it is given once, at mount, so that surviving canvas kept reporting the old drawing under the new file's name. Each drawing now saves through its own session, bound to the path it was read from, the canvas is remounted per path, and a canvas being torn down can no longer arm a write against the drawing that replaced it. The viewport memory that restores your pan and zoom per drawing follows the same binding, so a switch no longer copies the previous drawing's viewport onto the next one. + + How to test locally: build and launch the isolated desktop as described below. In the sidebar, create a drawing named **First** and draw a circle, then create **Second** and draw a rectangle. Click First, then Second, then First, pausing about a second on each. Before: within a second of a switch, the drawing you opened shows and saves the previous drawing's shape, and reopening either file confirms the loss. After: each drawing keeps its own shape through any number of switches, and the files on disk still hold one circle and one rectangle. The same holds when you switch immediately after drawing, before the save delay has run. + +- **A conflicted note keeps its tasks in view.** List, Calendar, Kanban and the calendar panel continue showing the tasks from your local note, with a small **Conflict pending** label. Previously those tasks were deliberately withheld until the note was resolved, so deferring a sync decision also hid work from your task views. They now remain visible and refresh when you edit the local note. Resolving the conflict updates them only as the resulting note changes. + + How to test locally: launch the isolated desktop as described below. Put a dated task in a note, sync it, then edit the same line differently on two linked test devices. Press **Sync now**, leave the conflict unresolved, and open Tasks in List, Calendar and Kanban. Before: the note's tasks disappear. After: they remain with **Conflict pending**. Resolve the note and confirm the tasks match the chosen content and the label clears. + +- **Identical files stop waiting for a decision.** On the next sync, ZenNotes checks the current local file against the latest tracked Cloud version. When the same file at the same path has identical content and no separate draft to preserve, the conflict clears, the review closes if nothing else is waiting, and later edits sync normally. It does not rewrite either copy or choose one device over the other. Real differences, path/rename/deletion conflicts and unfinished combined-note drafts still need review. + + How to test locally: create a conflict and leave **Review now** open. Copy the complete Cloud version into the local file, then sync. Before: identical versions still block the file. After: the review closes and the next edit uploads. Repeat after typing a distinct combined-note draft: the review and saved draft must remain. Open the same vault in a second desktop window and sync there; the draft stays intact, and only one window can edit that conflict at a time. + +## 🧰 For contributors + +- The drawing fix is commit `29c5287` on `v2.47.0` (#755 closed 2026-09-09). Verified in the built desktop app over CDP: with seeded drawings, switching alone corrupted a file in 4 of 6 runs before the fix and 0 of 6 after; with the reporter's flow driven by real mouse drags (draw, switch inside the save delay, draw, switch, then rapid switches), 3 of 3 runs corrupted before and 0 of 3 after. The unit tests in `ExcalidrawView.test.ts` model the batched read and the late canvas report; both fail on the previous view. The app's bridge object is frozen in the renderer, so the harness watched the files on disk and hooked the renderer's timers instead of spying on `writeNote`. +- The wikilink-file fix is commit `bee2284` on `v2.47.0` (#757 closed 2026-09-09). Verified in the built desktop app over CDP against the previous build: the Connections row for `assets/diagram.png` went from a CREATE row suggesting `/assets/diagram.png.md` to a FILE row, `j` then Enter on it opened the asset tab, and `gd` inside `[[assets/diagram.png]]` went from the create prompt to the image tab. Unit tests cover the outgoing-link classifier (`connections-outgoing.test.ts`) and the follow-link path. Harness note: a real CDP mouse click on a panel row fires no handler, before and after the fix, so the check drives rows through the keyboard. +- Build with `npm run build --workspace @zennotes/desktop`. Create a scratch root with `mktemp -d /tmp/zennotes-247.XXXXXX`, then launch `ZEN_PERF=1 ZENNOTES_USER_DATA_PATH=/userdata ZENNOTES_CONFIG_DIR=/config ZENNOTES_CLOUD_BASE_URL=http://127.0.0.1:43183 apps/desktop/node_modules/.bin/electron apps/desktop/out/main/index.js --remote-debugging-port=9326`. Use a scratch vault and local test account only. The existing `node tooling/scripts/cloud-conflict-demo-fixture.mjs` supplies the local Cloud server. +- Draft saves are flushed before sync takes the vault lock. Same-vault desktop windows acknowledge preparation and share the result; failed saves or a window opening/closing mid-preparation pause that run safely. Retry after the window loads. +- Verified in the built desktop UI against the local fixture: task visibility, live and persisted conflict convergence, subsequent upload, draft retention, restart and two-window review ownership. +- Release gates run fresh at the cut: `apps/desktop` `build:prod` (typecheck, tests, build, packaged CLI isolation), Go vet and tests, the app-core suite (1,980 tests), the signed packaged app launched in an isolated profile and reached a CDP page target in about 3 seconds, and the website suite (691 tests) with the release page entry in place. +- The high-severity audit findings for `js-yaml` and `svgo` were cleared on `main` by [#753](https://github.com/ZenNotes/zennotes/pull/753) and merged into this branch before the cut. One moderate finding (`hono`) remains and predates this release. + +--- + +Local-first and keyboard-first, as always. diff --git a/docs/releases/v2.47.0/media/connections-file-after-757.png b/docs/releases/v2.47.0/media/connections-file-after-757.png new file mode 100644 index 00000000..0bc845d0 Binary files /dev/null and b/docs/releases/v2.47.0/media/connections-file-after-757.png differ diff --git a/docs/releases/v2.47.0/media/connections-file-before-757.png b/docs/releases/v2.47.0/media/connections-file-before-757.png new file mode 100644 index 00000000..949a747d Binary files /dev/null and b/docs/releases/v2.47.0/media/connections-file-before-757.png differ diff --git a/docs/releases/v2.47.0/media/drawing-switch-after-755.png b/docs/releases/v2.47.0/media/drawing-switch-after-755.png new file mode 100644 index 00000000..1577a75e Binary files /dev/null and b/docs/releases/v2.47.0/media/drawing-switch-after-755.png differ diff --git a/docs/releases/v2.47.0/media/drawing-switch-before-755.png b/docs/releases/v2.47.0/media/drawing-switch-before-755.png new file mode 100644 index 00000000..f34bb52b Binary files /dev/null and b/docs/releases/v2.47.0/media/drawing-switch-before-755.png differ diff --git a/docs/releases/v2.47.0/twitter-post.md b/docs/releases/v2.47.0/twitter-post.md new file mode 100644 index 00000000..bd251c10 --- /dev/null +++ b/docs/releases/v2.47.0/twitter-post.md @@ -0,0 +1,33 @@ +# Twitter/X thread for ZenNotes 2.47.0 + +Draft for release day. Do not post until the release is published and the dependency-audit blocker is cleared. + +## Tweet 1 + +ZenNotes 2.47.0 fixes a data-loss bug in drawings: switching from one Excalidraw drawing to another could overwrite the one you opened with the one you left (#755). Each drawing now saves to its own file, no matter how fast you switch. If you hit this, please update. + +## Tweet 2 + +The Connections panel no longer mistakes an embedded image for a missing note (#757). A wikilink at a file in your vault now shows as a file and opens in its own tab, from the panel, from gd, and from a click. Creating a note is offered only when nothing answers the link. + +## Tweet 3 + +Also in 2.47.0: your tasks stay visible while a Cloud conflict waits. Calendar, Kanban and List show your local tasks with a small "Conflict pending" label. You can review the note later without losing sight of today's work. + +## Tweet 4 + +Made both devices' files identical by copying the whole note? The next sync now clears that conflict and lets the file sync normally again. Genuine differences still need your decision. Unfinished merge drafts are kept safe. + +## Tweet 5 + +Review drafts stay safe across desktop windows too. One window edits a conflict at a time, and sync waits for the latest draft to be saved. Thanks @uNyanda for the reports and gverger for #755. Release PR #752. + +Free, open source, local-first Markdown notes. +https://zennotes.org + +## Notes + +- Release PR: https://github.com/ZenNotes/zennotes/pull/752 +- No separate GitHub issue was created for these September 8 reports. Do not mark the older conflict-workflow issue #683 as newly closed. +- Desktop and shared-package fixes only; no Laravel website deployment or mobile release is included. +- No new demo video has been recorded for this release. diff --git a/package-lock.json b/package-lock.json index 79089abc..96ed272d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "zennotes-monorepo", - "version": "2.46.0", + "version": "2.47.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "zennotes-monorepo", - "version": "2.46.0", + "version": "2.47.0", "hasInstallScript": true, "workspaces": [ "apps/*", @@ -23,7 +23,7 @@ }, "apps/desktop": { "name": "@zennotes/desktop", - "version": "2.46.0", + "version": "2.47.0", "license": "MIT", "dependencies": { "@codemirror/autocomplete": "^6.18.3", @@ -874,11 +874,11 @@ }, "apps/server": { "name": "@zennotes/server", - "version": "2.46.0" + "version": "2.47.0" }, "apps/web": { "name": "@zennotes/web", - "version": "2.46.0", + "version": "2.47.0", "dependencies": { "@codemirror/autocomplete": "^6.18.3", "@codemirror/commands": "^6.7.1", @@ -16286,7 +16286,7 @@ }, "packages/app-core": { "name": "@zennotes/app-core", - "version": "2.46.0", + "version": "2.47.0", "dependencies": { "@codemirror/autocomplete": "^6.18.3", "@codemirror/commands": "^6.7.1", @@ -16363,11 +16363,11 @@ }, "packages/bridge-contract": { "name": "@zennotes/bridge-contract", - "version": "2.46.0" + "version": "2.47.0" }, "packages/shared-domain": { "name": "@zennotes/shared-domain", - "version": "2.46.0", + "version": "2.47.0", "dependencies": { "@zennotes/bridge-contract": "*", "lz-string": "^1.5.0" @@ -16378,7 +16378,7 @@ }, "packages/shared-ui": { "name": "@zennotes/shared-ui", - "version": "2.46.0" + "version": "2.47.0" } } } diff --git a/package.json b/package.json index 3a38ed97..13a92d91 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "zennotes-monorepo", "private": true, - "version": "2.46.0", + "version": "2.47.0", "description": "ZenNotes monorepo for desktop, web, and self-hosted server builds", "packageManager": "npm@10.9.2", "engines": { diff --git a/packages/app-core/package.json b/packages/app-core/package.json index eec3d378..4a410394 100644 --- a/packages/app-core/package.json +++ b/packages/app-core/package.json @@ -1,7 +1,7 @@ { "name": "@zennotes/app-core", "private": true, - "version": "2.46.0", + "version": "2.47.0", "type": "module", "exports": { "./main": "./src/main.tsx" diff --git a/packages/app-core/src/components/CalendarPanel.tsx b/packages/app-core/src/components/CalendarPanel.tsx index 73134ca4..c490085a 100644 --- a/packages/app-core/src/components/CalendarPanel.tsx +++ b/packages/app-core/src/components/CalendarPanel.tsx @@ -33,6 +33,7 @@ import { import { getISOWeek, getISOWeekYear } from '../lib/template-render' import { countWords } from '../lib/word-count' import { InlineMarkdown } from '../lib/inline-markdown' +import { CloudTaskConflictIndicator } from './CloudTaskConflictIndicator' import { resolveWeekStartDay } from '../lib/week-start' import { ChevronLeftIcon, ChevronRightIcon } from './icons' import { confirmApp } from '../lib/confirm-requests' @@ -853,6 +854,7 @@ export function CalendarPanel({ note }: { note: NoteContent }): JSX.Element { {task.sourcePath !== (dailyByDate.get(dayIso)?.path ?? '') && ( {task.noteTitle} )} + ) diff --git a/packages/app-core/src/components/CloudPendingConflictResolver.test.ts b/packages/app-core/src/components/CloudPendingConflictResolver.test.ts index 76ef68ee..ab50f854 100644 --- a/packages/app-core/src/components/CloudPendingConflictResolver.test.ts +++ b/packages/app-core/src/components/CloudPendingConflictResolver.test.ts @@ -9,9 +9,15 @@ import type { CloudSyncRunSummary, } from "@zennotes/bridge-contract/cloud-sync"; import { CloudPendingConflictResolver } from "./CloudPendingConflictResolver"; +import { + clearCloudSyncStatus, + syncCloudVaultWithStatus, + useCloudSyncStatusStore, +} from "../lib/cloud-auto-sync"; const bridge = vi.hoisted(() => ({ getCloudConflict: vi.fn(), + releaseCloudConflictReview: vi.fn(), saveCloudConflictDraft: vi.fn(), resolveCloudConflict: vi.fn(), syncCloudVault: vi.fn(), @@ -67,13 +73,79 @@ const synced: CloudSyncRunSummary = { }; beforeEach(() => { + clearCloudSyncStatus(); bridge.getCloudConflict.mockReset().mockResolvedValue(details); + bridge.releaseCloudConflictReview.mockReset().mockResolvedValue(undefined); bridge.saveCloudConflictDraft.mockReset().mockResolvedValue(undefined); bridge.resolveCloudConflict.mockReset().mockResolvedValue(undefined); bridge.syncCloudVault.mockReset().mockResolvedValue(synced); }); describe("CloudPendingConflictResolver", () => { + it("releases its own review session after unmount even if the final draft save fails", async () => { + const view = mount({}); + await act(async () => Promise.resolve()); + const reviewId = bridge.getCloudConflict.mock.calls[0][1]; + expect(typeof reviewId).toBe("string"); + await act(async () => button(view.host, "Use other device").click()); + bridge.saveCloudConflictDraft.mockRejectedValue(new Error("Disk is full")); + view.unmount(); + await act(async () => { await Promise.resolve(); await Promise.resolve(); }); + expect(bridge.releaseCloudConflictReview).toHaveBeenCalledWith(conflict.id, reviewId); + }); + + it("flushes a freshly edited draft before background sync and locks editing until it finishes", async () => { + let finishSave!: () => void; + bridge.saveCloudConflictDraft.mockImplementationOnce( + () => new Promise((resolve) => { finishSave = resolve; }), + ); + const pending = { ...synced, pending_conflicts: [conflict] }; + bridge.syncCloudVault.mockResolvedValue(pending); + useCloudSyncStatusStore.setState({ conflictReviewOpen: true }); + const view = mount({}); + await act(async () => Promise.resolve()); + + await act(async () => { + const editor = textarea(view.host); + Object.getOwnPropertyDescriptor(HTMLTextAreaElement.prototype, "value") + ?.set?.call(editor, "An unfinished third version\n"); + editor.dispatchEvent(new Event("input", { bubbles: true })); + }); + expect(bridge.saveCloudConflictDraft).not.toHaveBeenCalled(); + + let run!: Promise; + await act(async () => { run = syncCloudVaultWithStatus(bridge); }); + expect(bridge.saveCloudConflictDraft).toHaveBeenCalledWith( + conflict.id, "An unfinished third version\n", + ); + expect(bridge.syncCloudVault).not.toHaveBeenCalled(); + expect(textarea(view.host).disabled).toBe(true); + + await act(async () => { finishSave(); await run; }); + expect(bridge.syncCloudVault).toHaveBeenCalledOnce(); + expect(textarea(view.host).disabled).toBe(false); + expect(textarea(view.host).value).toBe("An unfinished third version\n"); + expect(useCloudSyncStatusStore.getState().conflictReviewOpen).toBe(true); + view.unmount(); + }); + + it("keeps an unsaved draft and the review open when its pre-sync save fails", async () => { + bridge.saveCloudConflictDraft.mockRejectedValue(new Error("Disk is full")); + useCloudSyncStatusStore.setState({ conflictReviewOpen: true }); + const view = mount({}); + await act(async () => Promise.resolve()); + await act(async () => button(view.host, "Use other device").click()); + await act(async () => { + await expect(syncCloudVaultWithStatus(bridge)).rejects.toThrow("Disk is full"); + }); + expect(bridge.syncCloudVault).not.toHaveBeenCalled(); + expect(textarea(view.host).value).toBe("# Trip\nPack a rain coat.\n"); + expect(textarea(view.host).disabled).toBe(false); + expect(view.host.textContent).toContain("Disk is full"); + expect(useCloudSyncStatusStore.getState().conflictReviewOpen).toBe(true); + view.unmount(); + }); + it("uses plain labels and requires an explicit choice for overlapping text", async () => { const onResolved = vi.fn(); const view = mount({ onResolved }); diff --git a/packages/app-core/src/components/CloudPendingConflictResolver.tsx b/packages/app-core/src/components/CloudPendingConflictResolver.tsx index 420958fb..b77b2fec 100644 --- a/packages/app-core/src/components/CloudPendingConflictResolver.tsx +++ b/packages/app-core/src/components/CloudPendingConflictResolver.tsx @@ -7,7 +7,11 @@ import type { CloudSyncRunSummary, } from "@zennotes/bridge-contract/cloud-sync"; import { getZenBridge } from "@zennotes/bridge-contract/bridge"; -import { syncCloudVaultWithStatus } from "../lib/cloud-auto-sync"; +import { + registerCloudConflictDraftFlusher, + syncCloudVaultWithStatus, + useCloudSyncStatusStore, +} from "../lib/cloud-auto-sync"; import { Button } from "./ui/Button"; type ChangeChoice = "local" | "cloud" | "both"; @@ -33,7 +37,9 @@ export function CloudPendingConflictResolver({ const [saveState, setSaveState] = useState<"idle" | "saving" | "saved">( "idle", ); - const [busy, setBusy] = useState(false); + const [resolving, setBusy] = useState(false); + const syncing = useCloudSyncStatusStore((state) => state.phase === "syncing" || state.syncWindowLocked); + const busy = resolving || syncing; const [error, setError] = useState(null); const [keepBothOpen, setKeepBothOpen] = useState(false); const [finishLaterOpen, setFinishLaterOpen] = useState(false); @@ -50,6 +56,9 @@ export function CloudPendingConflictResolver({ const [reloading, setReloading] = useState(false); const [resolvedPath, setResolvedPath] = useState(conflict.path); const loadedDraft = useRef(null); + const latestDraft = useRef(""); + const reviewId = useRef(crypto.randomUUID()); + const reviewGeneration = useRef(0); const finishLaterButton = useRef(null); const finishLaterDialog = useRef(null); const wholeVersionSource = useRef(null); @@ -66,7 +75,7 @@ export function CloudPendingConflictResolver({ setCombineOpen(false); setWholeVersionChoice(null); void bridge - .getCloudConflict(conflict.id) + .getCloudConflict(conflict.id, reviewId.current) .then((next) => { if (cancelled) return; setReloading(false); @@ -79,6 +88,7 @@ export function CloudPendingConflictResolver({ next.cloud.text ?? "")); loadedDraft.current = initialDraft; + latestDraft.current = initialDraft; setDetails(next); setResolvedPath(next.local.path ?? next.cloud.path ?? conflict.path); setDraft(initialDraft); @@ -95,6 +105,35 @@ export function CloudPendingConflictResolver({ }; }, [bridge, conflict.id, reload]); + async function flushDraft(): Promise { + const value = latestDraft.current; + if (loadedDraft.current === null || value === loadedDraft.current) return; + setSaveState("saving"); + try { + await bridge.saveCloudConflictDraft(conflict.id, value); + loadedDraft.current = value; + setSaveState("saved"); + } catch (cause) { + setSaveState("idle"); + setError(message(cause)); + throw cause; + } + } + + useEffect(() => { + const generation = ++reviewGeneration.current; + const unregister = registerCloudConflictDraftFlusher(flushDraft); + return () => { + // A different window may review this file only after our final edit is saved. + void flushDraft().catch(() => {}).finally(() => { + if (reviewGeneration.current === generation) { + return bridge.releaseCloudConflictReview?.(conflict.id, reviewId.current); + } + }).catch(() => {}); + unregister(); + }; + }, [bridge, conflict.id]); + useEffect(() => { if (finishLaterOpen) finishLaterDialog.current?.focus(); }, [finishLaterOpen]); @@ -107,16 +146,7 @@ export function CloudPendingConflictResolver({ if (!details || draft === loadedDraft.current) return undefined; setSaveState("saving"); const timeout = window.setTimeout(() => { - void bridge - .saveCloudConflictDraft(conflict.id, draft) - .then(() => { - loadedDraft.current = draft; - setSaveState("saved"); - }) - .catch((cause) => { - setSaveState("idle"); - setError(message(cause)); - }); + void flushDraft().catch(() => {}); }, 500); return () => window.clearTimeout(timeout); }, [bridge, conflict.id, details, draft]); @@ -147,7 +177,8 @@ export function CloudPendingConflictResolver({ setWholeVersionChoice(null); const nextChoices = { ...choices, [changeId]: choice }; setChoices(nextChoices); - setDraft(combinedText(details, nextChoices)); + latestDraft.current = combinedText(details, nextChoices); + setDraft(latestDraft.current); setManualDraft(false); }; @@ -175,6 +206,7 @@ export function CloudPendingConflictResolver({ setBusy(true); setError(null); try { + await flushDraft(); await bridge.resolveCloudConflict({ conflict_id: conflict.id, expected_local_sha256: details.local.sha256, @@ -192,10 +224,8 @@ export function CloudPendingConflictResolver({ const finishLater = async (): Promise => { if (details && draft !== loadedDraft.current) { - setSaveState("saving"); try { - await bridge.saveCloudConflictDraft(conflict.id, draft); - loadedDraft.current = draft; + await flushDraft(); } catch (cause) { setError(message(cause)); setSaveState("idle"); @@ -398,6 +428,7 @@ export function CloudPendingConflictResolver({ value={draft} disabled={busy} onChange={(event) => { + latestDraft.current = event.target.value; setDraft(event.target.value); setManualDraft(true); }} diff --git a/packages/app-core/src/components/CloudTaskConflictIndicator.test.ts b/packages/app-core/src/components/CloudTaskConflictIndicator.test.ts new file mode 100644 index 00000000..86f78ef9 --- /dev/null +++ b/packages/app-core/src/components/CloudTaskConflictIndicator.test.ts @@ -0,0 +1,57 @@ +// @vitest-environment jsdom +import { act, createElement } from "react"; +import { createRoot } from "react-dom/client"; +import { afterEach, expect, it } from "vitest"; +import { + clearCloudSyncStatus, + useCloudSyncStatusStore, +} from "../lib/cloud-auto-sync"; +import { CloudTaskConflictIndicator } from "./CloudTaskConflictIndicator"; +( + globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean } +).IS_REACT_ACT_ENVIRONMENT = true; +afterEach(clearCloudSyncStatus); + +it("labels only tasks in a pending note and clears the label when it is resolved", async () => { + const host = document.createElement("div"); + const root = createRoot(host); + await act(async () => { + useCloudSyncStatusStore.setState({ + lastSummary: { + cursor: 2, + pulled: 1, + pushed: 0, + conflicts: [], + bootstrap_conflicts: [], + local_conflicts: [], + pending_conflicts: [ + { + id: "note", + item_id: "note", + path: "Plan.md", + cloud_path: "Moved.md", + kind: "move", + can_merge: false, + has_base: true, + }, + ], + }, + }); + root.render( + createElement( + "div", + null, + createElement(CloudTaskConflictIndicator, { path: "Plan.md" }), + createElement(CloudTaskConflictIndicator, { path: "Moved.md" }), + createElement(CloudTaskConflictIndicator, { path: "Other.md" }), + ), + ); + }); + expect(host.textContent).toBe("Conflict pendingConflict pending"); + expect(host.querySelector("[title]")?.getAttribute("title")).toContain( + "local note", + ); + await act(async () => clearCloudSyncStatus()); + expect(host.textContent).toBe(""); + await act(async () => root.unmount()); +}); diff --git a/packages/app-core/src/components/CloudTaskConflictIndicator.tsx b/packages/app-core/src/components/CloudTaskConflictIndicator.tsx new file mode 100644 index 00000000..e5dbb507 --- /dev/null +++ b/packages/app-core/src/components/CloudTaskConflictIndicator.tsx @@ -0,0 +1,26 @@ +import { cloudSyncPathKey } from "@zennotes/shared-domain/cloud-sync"; +import { useCloudSyncStatusStore } from "../lib/cloud-auto-sync"; + +export function CloudTaskConflictIndicator({ + path, +}: { + path: string; +}): JSX.Element | null { + const pending = useCloudSyncStatusStore((state) => + (state.lastSummary?.pending_conflicts ?? []).some((conflict) => + [conflict.path, conflict.cloud_path].some( + (candidate) => + candidate && cloudSyncPathKey(candidate) === cloudSyncPathKey(path), + ), + ), + ); + if (!pending) return null; + return ( + + Conflict pending + + ); +} diff --git a/packages/app-core/src/components/ConnectionsPanel.tsx b/packages/app-core/src/components/ConnectionsPanel.tsx index 8049c5a9..66290a3d 100644 --- a/packages/app-core/src/components/ConnectionsPanel.tsx +++ b/packages/app-core/src/components/ConnectionsPanel.tsx @@ -7,10 +7,11 @@ import { extractMarkdownLinkHrefs, extractMentionSnippet, parseCreateNotePath, - resolveWikilinkTarget, - suggestCreateNotePath + resolveWikilinkTarget } from '../lib/wikilinks' import { resolveInternalNoteHref } from '../lib/internal-links' +import { classifyOutgoingWikilinks, type AttachmentLink } from '../lib/connections-outgoing' +import { assetTabPath } from '../lib/asset-tabs' import { LazyNoteHoverPreview as NoteHoverPreview } from './LazyNoteHoverPreview' import { promptApp } from '../lib/prompt-requests' import { usePanelResize } from '../lib/use-panel-resize' @@ -36,6 +37,8 @@ export function ConnectionsPanel({ note }: { note: NoteContent }): JSX.Element { const notes = useStore((s) => s.notes) const selectNote = useStore((s) => s.selectNote) const createAndOpen = useStore((s) => s.createAndOpen) + const openNoteInTab = useStore((s) => s.openNoteInTab) + const assetFiles = useStore((s) => s.assetFiles) const panelWidth = useStore((s) => s.panelWidths.connections) const setPanelWidth = useStore((s) => s.setPanelWidth) const { startResize } = usePanelResize(panelWidth, (px) => setPanelWidth('connections', px)) @@ -86,32 +89,14 @@ export function ConnectionsPanel({ note }: { note: NoteContent }): JSX.Element { } const outgoing = useMemo(() => { - const targets = extractWikilinkTargets(note.body) - const seen = new Set() - const resolvedItems: NoteMeta[] = [] - const missingItems: MissingLinkItem[] = [] - for (const rawTarget of targets) { - const target = rawTarget.trim() - if (!target) continue - const dedupeKey = target.toLowerCase() - if (seen.has(dedupeKey)) continue - seen.add(dedupeKey) - const resolved = resolveWikilinkTarget(notes, target) - if (!resolved) continue - if (resolved.folder === 'trash' || resolved.path === note.path) continue - resolvedItems.push(resolved) - } - for (const rawTarget of targets) { - const target = rawTarget.trim() - if (!target) continue - const resolved = resolveWikilinkTarget(notes, target) - if (resolved || target.toLowerCase() === note.title.toLowerCase()) continue - if (missingItems.some((item) => item.target.toLowerCase() === target.toLowerCase())) continue - missingItems.push({ - target, - suggestedPath: suggestCreateNotePath(target) - }) - } + const links = classifyOutgoingWikilinks({ + body: note.body, + notePath: note.path, + noteTitle: note.title, + notes, + assets: assetFiles + }) + const resolvedItems = links.resolved // #70dark: standard Markdown links [text](Note.md) also count as outgoing // connections — resolve each href the way `gd` does and add resolved notes. for (const href of extractMarkdownLinkHrefs(note.body)) { @@ -121,11 +106,12 @@ export function ConnectionsPanel({ note }: { note: NoteContent }): JSX.Element { const target = notes.find((n) => n.path === resolvedPath) if (target && target.folder !== 'trash') resolvedItems.push(target) } - return { resolvedItems, missingItems } - }, [note.body, note.path, notes]) + return { resolvedItems, attachmentItems: links.attachments, missingItems: links.missing } + }, [assetFiles, note.body, note.path, note.title, notes]) const totalRows = outgoing.resolvedItems.length + + outgoing.attachmentItems.length + outgoing.missingItems.length + backlinks.length + mentions.length @@ -265,7 +251,12 @@ export function ConnectionsPanel({ note }: { note: NoteContent }): JSX.Element { Connections
- {outgoing.resolvedItems.length + outgoing.missingItems.length} out + + {outgoing.resolvedItems.length + + outgoing.attachmentItems.length + + outgoing.missingItems.length}{' '} + out + {backlinks.length} in {mentions.length} mentioned
@@ -286,7 +277,7 @@ export function ConnectionsPanel({ note }: { note: NoteContent }): JSX.Element {
setConnectionPreview(null)}> {outgoing.resolvedItems.map((item) => ( @@ -304,6 +295,15 @@ export function ConnectionsPanel({ note }: { note: NoteContent }): JSX.Element { rowIndex={rowIndex++} /> ))} + {outgoing.attachmentItems.map((item) => ( + void openNoteInTab(assetTabPath(item.assetPath))} + active={isConnectionsFocused && connectionsCursorIndex === rowIndex} + rowIndex={rowIndex++} + /> + ))} {outgoing.missingItems.map((item) => ( void + active: boolean + rowIndex: number +}): JSX.Element { + const name = link.assetPath.split('/').pop() ?? link.assetPath + return ( + + ) +} + function MissingConnectionRow({ target, suggestedPath, diff --git a/packages/app-core/src/components/Editor.tsx b/packages/app-core/src/components/Editor.tsx index 6616b3dd..34a4bc11 100644 --- a/packages/app-core/src/components/Editor.tsx +++ b/packages/app-core/src/components/Editor.tsx @@ -52,6 +52,7 @@ import { } from "../lib/move-note"; import { promptApp } from "../lib/prompt-requests"; import { offerCreateNoteFromLink } from "../lib/create-note-from-link"; +import { openWikilinkAttachment } from "../lib/open-wikilink-attachment"; import { externalFileLink, openExternalFileLink, @@ -850,6 +851,10 @@ function registerVimCommands(): void { return; } + // A file in the vault (an embedded image, a non-PDF attachment) opens in + // its own tab instead of becoming a create offer for `file.png.md`. (#757) + if (openWikilinkAttachment(target)) return; + // A link to a file outside the vault: open it with the OS default app. (#424) if (externalFileLink(target)) { void openExternalFileLink(target); diff --git a/packages/app-core/src/components/ExcalidrawView.test.ts b/packages/app-core/src/components/ExcalidrawView.test.ts index 8dbe82bf..573711ab 100644 --- a/packages/app-core/src/components/ExcalidrawView.test.ts +++ b/packages/app-core/src/components/ExcalidrawView.test.ts @@ -1,27 +1,38 @@ // @vitest-environment jsdom // -// Guards: ExcalidrawView derives its theme from document.documentElement.dataset.themeMode -// rather than looking up the theme id in THEMES. The THEMES array only contains built-in -// themes, so a custom theme id (e.g. "custom-mine") would always resolve to "light" under -// the old approach — THEMES.find returns undefined and undefined?.mode === 'dark' is false. -// This test verifies that dark custom themes correctly produce a dark Excalidraw theme, -// and would catch a regression back to THEMES.find(). - -import { act, createElement } from 'react' +// Guards two things about ExcalidrawView. +// +// Theme: the view derives its theme from document.documentElement.dataset.themeMode +// rather than looking up the theme id in THEMES. The THEMES array only contains +// built-in themes, so a custom theme id (e.g. "custom-mine") would always resolve +// to "light" under the old approach (THEMES.find returns undefined and +// undefined?.mode === 'dark' is false). Those tests would catch a regression back +// to THEMES.find(). +// +// Saving: a scene the canvas reports is written to the path that canvas was +// opened for, never to the path the view happens to show when the debounce +// fires (#755). The stand-in canvas below reproduces the two Excalidraw +// behaviours the view has to survive: initialData is read once at mount (a +// keyed remount gets a fresh scene, a prop change does not), and onChange +// fires from componentDidUpdate on every render with the scene the canvas +// holds. + +import { act, createElement, useLayoutEffect, useState } from 'react' import { createRoot, type Root } from 'react-dom/client' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -// Spy on the Excalidraw component so we can inspect its theme prop. -// vi.hoisted is needed so the variable is in scope for both the mock module factory -// and the test body. -const { mockExcalidraw } = vi.hoisted(() => { - const mockExcalidraw = vi.fn((_props: Record) => null) - return { mockExcalidraw } +// Spy on the Excalidraw component so we can inspect its props. vi.hoisted is +// needed so the variables are in scope for both the mock module factory and +// the test body. +const { mockExcalidraw, mockSerializeAsJSON } = vi.hoisted(() => { + const mockExcalidraw = vi.fn((_props: Record): null => null) + const mockSerializeAsJSON = vi.fn((elements: unknown): string => JSON.stringify(elements)) + return { mockExcalidraw, mockSerializeAsJSON } }) vi.mock('@excalidraw/excalidraw', () => ({ Excalidraw: mockExcalidraw, - serializeAsJSON: vi.fn(() => '{}') + serializeAsJSON: mockSerializeAsJSON })) // Mock store: expose a custom theme id that does NOT exist in the built-in THEMES array. @@ -29,6 +40,7 @@ const { storeState } = vi.hoisted(() => { const storeState: Record = { themeId: 'custom-test-theme', themeMode: 'dark', + setFocusedPanel: () => undefined } return { storeState } }) @@ -39,6 +51,34 @@ vi.mock('../store', () => ({ import { ExcalidrawView } from './ExcalidrawView' +type SceneElement = { id: string; type: string } +type CanvasProps = { + initialData?: { elements?: SceneElement[] } + onChange?: (elements: SceneElement[], appState: unknown, files: unknown) => void + theme?: string +} + +const CANVAS_APP_STATE = { scrollX: 0, scrollY: 0, zoom: { value: 1 } } + +function FakeCanvas(props: CanvasProps): null { + const [scene] = useState(() => props.initialData?.elements ?? []) + useLayoutEffect(() => { + props.onChange?.(scene, CANVAS_APP_STATE, {}) + }) + return null +} +mockExcalidraw.mockImplementation(FakeCanvas as (props: Record) => null) + +const FIRST = 'inbox/First.excalidraw' +const SECOND = 'inbox/Second.excalidraw' +const ELLIPSE: SceneElement[] = [{ id: 'first-ellipse', type: 'ellipse' }] +const RECTANGLE: SceneElement[] = [{ id: 'second-rect', type: 'rectangle' }] +const SAVE_DEBOUNCE_MS = 700 + +function drawingBody(elements: SceneElement[]): string { + return JSON.stringify({ type: 'excalidraw', version: 2, elements, appState: {}, files: {} }) +} + describe('ExcalidrawView theme mode with custom themes', () => { let root: Root let host: HTMLDivElement @@ -50,7 +90,7 @@ describe('ExcalidrawView theme mode with custom themes', () => { configurable: true, value: { readNote: vi.fn().mockResolvedValue({ body: '{}' }), - writeNote: vi.fn(), + writeNote: vi.fn() } }) host = document.createElement('div') @@ -69,7 +109,7 @@ describe('ExcalidrawView theme mode with custom themes', () => { await act(async () => { root.render(createElement(ExcalidrawView, { path: 'inbox/drawing.excalidraw' })) }) - // Flush the readNote effect so setInitialData runs and Excalidraw renders. + // Flush the readNote effect so the drawing loads and Excalidraw renders. await act(async () => {}) expect(mockExcalidraw).toHaveBeenCalled() @@ -109,3 +149,125 @@ describe('ExcalidrawView theme mode with custom themes', () => { expect(mockExcalidraw.mock.lastCall?.[0].theme).toBe('dark') }) }) + +describe('ExcalidrawView saves each drawing to its own path (#755)', () => { + let root: Root + let host: HTMLDivElement + let readNote: ReturnType + let writeNote: ReturnType + + const bodies: Record = { + [FIRST]: drawingBody(ELLIPSE), + [SECOND]: drawingBody(RECTANGLE) + } + + /** Every (path, elements) pair handed to writeNote, decoded. */ + const writes = (): Array<[string, SceneElement[]]> => + writeNote.mock.calls.map(([path, json]) => [path as string, JSON.parse(json as string)]) + + const render = (path: string): Promise => + act(async () => { + root.render(createElement(ExcalidrawView, { path })) + }) + + const settle = (): Promise => act(async () => {}) + + const passDebounce = (): Promise => + act(async () => { + vi.advanceTimersByTime(SAVE_DEBOUNCE_MS + 1) + }) + + beforeEach(() => { + vi.clearAllMocks() + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }) + ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + readNote = vi.fn((path: string) => Promise.resolve({ body: bodies[path] ?? '' })) + writeNote = vi.fn(() => Promise.resolve()) + Object.defineProperty(window, 'zen', { + configurable: true, + value: { readNote, writeNote } + }) + host = document.createElement('div') + document.body.append(host) + root = createRoot(host) + }) + + afterEach(() => { + act(() => root.unmount()) + host.remove() + vi.useRealTimers() + }) + + it('writes the drawing being left to its own path and never over the next one', async () => { + await render(FIRST) + await settle() + await passDebounce() + expect(writes()).toEqual([[FIRST, ELLIPSE]]) + + await render(SECOND) + await settle() + await passDebounce() + + for (const [path, elements] of writes()) { + expect(elements, `${path} received the wrong scene`).toEqual( + path === FIRST ? ELLIPSE : RECTANGLE + ) + } + expect(writes().some(([path]) => path === SECOND)).toBe(true) + }) + + it('keeps the previous scene off the new path when the next read lands in the same render batch', async () => { + await render(FIRST) + await settle() + await passDebounce() + + // A local vault answers the read for the next drawing before React gets to + // render the "Loading drawing" placeholder, so both state updates land in + // one batch and the canvas is never torn down between the two drawings. + // A thenable that settles synchronously reproduces that batch exactly. + readNote.mockImplementation((path: string) => { + const settled = { + then(onFulfilled: (value: { body: string }) => void) { + onFulfilled({ body: bodies[path] ?? '' }) + return settled + }, + catch() { + return settled + } + } + return settled + }) + await render(SECOND) + await settle() + await passDebounce() + + const toSecond = writes().filter(([path]) => path === SECOND) + expect(toSecond.map(([, elements]) => elements)).not.toContainEqual(ELLIPSE) + expect(mockExcalidraw.mock.lastCall?.[0].initialData).toMatchObject({ elements: RECTANGLE }) + expect(bodies[FIRST]).toBe(drawingBody(ELLIPSE)) + }) + + it('ignores a change the torn-down canvas reports after the view moved on', async () => { + await render(FIRST) + await settle() + await passDebounce() + const firstCanvasOnChange = mockExcalidraw.mock.lastCall?.[0].onChange as CanvasProps['onChange'] + expect(firstCanvasOnChange).toBeTypeOf('function') + + await render(SECOND) + await settle() + const writesBefore = writes().length + + // Excalidraw calls onChange from componentDidUpdate, so a canvas that is + // being replaced can still report its scene once React has already moved + // the view to the next path. + await act(async () => { + firstCanvasOnChange?.(ELLIPSE, CANVAS_APP_STATE, {}) + }) + await passDebounce() + + // The late report produced nothing; the only write since is Second's own + // debounce carrying Second's scene. + expect(writes().slice(writesBefore)).toEqual([[SECOND, RECTANGLE]]) + }) +}) diff --git a/packages/app-core/src/components/ExcalidrawView.tsx b/packages/app-core/src/components/ExcalidrawView.tsx index 9aa7fe64..c15ef989 100644 --- a/packages/app-core/src/components/ExcalidrawView.tsx +++ b/packages/app-core/src/components/ExcalidrawView.tsx @@ -18,6 +18,44 @@ interface LatestScene { files: BinaryFiles } +/** + * One opened drawing: the scene the canvas reports and the debounced write + * that persists it, bound to the vault path the scene was read from. + * + * The binding is what keeps a switch between drawings from destroying one + * of them (#755). The view used to hold a single "latest scene" and write it + * to whichever path the view was showing when the debounce fired. Excalidraw + * reads `initialData` once at mount and reports its scene through onChange + * from componentDidUpdate on every render, so a canvas that survives a path + * change keeps reporting the OLD drawing under the NEW path. It survived more + * often than not: the read for the next drawing answers in about a + * millisecond on a local vault, and React then folds the "show the + * placeholder" and "mount the next drawing" state updates into a single + * render, so the old canvas was never torn down. Seven hundred milliseconds + * later the old scene overwrote the file that had just been opened. + * + * Every change is now recorded against the session of the canvas that + * reported it, and a session that the view has left is closed: its flush is + * final, and a last report from the canvas being torn down cannot arm a + * write against any path. + */ +interface DrawingSession { + path: string + /** Serialized scene last written to, or read from, `path`. */ + lastSaved: string + latest: LatestScene | null + timer: ReturnType | null + /** Set once the view leaves this drawing. */ + closed: boolean +} + +interface LoadedDrawing { + session: DrawingSession + initialData: InitialData +} + +const SAVE_DEBOUNCE_MS = 700 + type ViewportState = Pick const VIEWPORT_MEMORY_LIMIT = 60 @@ -37,6 +75,51 @@ function rememberViewport(path: string, appState: AppState): void { } } +function writeSession(session: DrawingSession): void { + const scene = session.latest + if (!scene) return + let json: string + try { + json = serializeAsJSON(scene.elements, scene.appState, scene.files, 'local') + } catch { + return + } + if (json === session.lastSaved) return + session.lastSaved = json + void window.zen.writeNote(session.path, json) +} + +function flushSession(session: DrawingSession): void { + if (session.timer) { + clearTimeout(session.timer) + session.timer = null + } + writeSession(session) +} + +function closeSession(session: DrawingSession | null): void { + if (!session || session.closed) return + session.closed = true + flushSession(session) +} + +function recordChange( + session: DrawingSession, + elements: SceneElements, + appState: AppState, + files: BinaryFiles +): void { + if (session.closed) return + session.latest = { elements, appState, files } + rememberViewport(session.path, appState) + if (session.timer) clearTimeout(session.timer) + session.timer = setTimeout(() => { + session.timer = null + // Skip no-op writes (Excalidraw fires onChange on load and on hover). + writeSession(session) + }, SAVE_DEBOUNCE_MS) +} + function readThemeMode(): 'light' | 'dark' { return typeof document !== 'undefined' && document.documentElement.dataset.themeMode === 'dark' @@ -50,35 +133,9 @@ function readThemeMode(): 'light' | 'dark' { * scene JSON from disk on open and debounce-saves it back on every change. */ export function ExcalidrawView({ path }: { path: string }): JSX.Element { - const [initialData, setInitialData] = useState(undefined) + const [loaded, setLoaded] = useState(null) const setFocusedPanel = useStore((s) => s.setFocusedPanel) - const saveTimer = useRef | null>(null) - const lastSaved = useRef('') - const latestScene = useRef(null) - const pathRef = useRef(path) - pathRef.current = path - - const writeLatestScene = (savePath: string): void => { - const scene = latestScene.current - if (!scene) return - let json: string - try { - json = serializeAsJSON(scene.elements, scene.appState, scene.files, 'local') - } catch { - return - } - if (json === lastSaved.current) return - lastSaved.current = json - void window.zen.writeNote(savePath, json) - } - - const flushPendingSave = (savePath = pathRef.current): void => { - if (saveTimer.current) { - clearTimeout(saveTimer.current) - saveTimer.current = null - } - writeLatestScene(savePath) - } + const sessionRef = useRef(null) // Follow the app's resolved light/dark mode. That mode lives on // ``, maintained in App.tsx (it already accounts for @@ -99,40 +156,49 @@ export function ExcalidrawView({ path }: { path: string }): JSX.Element { useEffect(() => { let cancelled = false - setInitialData(undefined) - window.zen - .readNote(path) - .then((res) => { - if (cancelled) return - lastSaved.current = res?.body ?? '' - latestScene.current = null - const doc = parseExcalidrawDocument(res?.body ?? '') - const rememberedViewport = viewportMemory.get(path) - setInitialData({ + setLoaded(null) + const open = (body: string): void => { + const doc = parseExcalidrawDocument(body) + const session: DrawingSession = { + path, + lastSaved: body, + latest: null, + timer: null, + closed: false + } + sessionRef.current = session + const rememberedViewport = viewportMemory.get(path) + setLoaded({ + session, + initialData: { elements: doc.elements, appState: rememberedViewport ? { ...doc.appState, ...rememberedViewport } : doc.appState, files: doc.files - } as InitialData) - }) - .catch(() => { - if (!cancelled) setInitialData({} as InitialData) + } as InitialData }) + } + window.zen.readNote(path).then( + (res) => { + if (!cancelled) open(res?.body ?? '') + }, + () => { + if (!cancelled) open('') + } + ) return () => { - flushPendingSave(path) cancelled = true + // Only a read that resolved for this path can have opened a session, so + // whatever is here belongs to the drawing being left. Closing it writes + // its scene to its own path and retires it before the next drawing is + // read; the view never carries a scene across paths. + closeSession(sessionRef.current) + sessionRef.current = null } }, [path]) - useEffect( - () => () => { - flushPendingSave() - }, - [] - ) - - if (initialData === undefined) { + if (!loaded) { return (
Loading drawing… @@ -140,6 +206,7 @@ export function ExcalidrawView({ path }: { path: string }): JSX.Element { ) } + const { session, initialData } = loaded return ( // Clicking into the drawing claims the keyboard the same way the editor // and the archive list do. Without the claim, focusedPanel stayed on the @@ -153,19 +220,16 @@ export function ExcalidrawView({ path }: { path: string }): JSX.Element { onMouseDownCapture={() => setFocusedPanel('editor')} onFocusCapture={() => setFocusedPanel('editor')} > + {/* One canvas per path. Excalidraw reads initialData once at mount, so a + path change has to remount it, including when React folds the + placeholder render into the next drawing's render (#755). */} { - latestScene.current = { elements, appState, files } - rememberViewport(pathRef.current, appState) - if (saveTimer.current) clearTimeout(saveTimer.current) - saveTimer.current = setTimeout(() => { - saveTimer.current = null - // Skip no-op writes (Excalidraw fires onChange on load and on hover). - writeLatestScene(pathRef.current) - }, 700) - }} + onChange={(elements, appState, files) => + recordChange(session, elements, appState, files) + } />
) diff --git a/packages/app-core/src/components/TasksCalendar.tsx b/packages/app-core/src/components/TasksCalendar.tsx index c65cc115..ba699b1e 100644 --- a/packages/app-core/src/components/TasksCalendar.tsx +++ b/packages/app-core/src/components/TasksCalendar.tsx @@ -23,6 +23,7 @@ import { import { useStore } from '../store' import { ChevronLeftIcon, ChevronRightIcon } from './icons' import { InlineMarkdown } from '../lib/inline-markdown' +import { CloudTaskConflictIndicator } from './CloudTaskConflictIndicator' import { resolveWeekStartDay } from '../lib/week-start' import { ContextMenu, type ContextMenuItem } from './ContextMenu' import { buildTaskMenuItems } from '../lib/task-context-menu' @@ -921,6 +922,7 @@ function CalendarTaskRow({ )} {task.noteTitle} + {task.priority && (
{task.noteTitle} + {task.priority && (
{task.noteTitle} + {task.waiting && ( @waiting diff --git a/packages/app-core/src/components/VimNav.tsx b/packages/app-core/src/components/VimNav.tsx index 80fad42f..0c397e3b 100644 --- a/packages/app-core/src/components/VimNav.tsx +++ b/packages/app-core/src/components/VimNav.tsx @@ -2291,7 +2291,7 @@ export function VimNav(): JSX.Element | null { }) return } - if (type === 'missing') { + if (type === 'missing' || type === 'attachment') { el.click() } } diff --git a/packages/app-core/src/lib/asset-path-resolution.ts b/packages/app-core/src/lib/asset-path-resolution.ts new file mode 100644 index 00000000..be151f9e --- /dev/null +++ b/packages/app-core/src/lib/asset-path-resolution.ts @@ -0,0 +1,103 @@ +/** + * Resolves an href or wikilink target to the vault-relative path of an existing + * asset, given the asset list. Store-free on purpose: surfaces that already + * hold the list (the Connections panel's outgoing links, the follow-link path) + * resolve without a store round trip, and the rules are unit-tested without + * booting the store. `resolveAssetVaultRelativePath` in local-assets.ts wraps + * this with the live `assetFiles`. + * + * Three readings, tried in order, each matching a way people write links: + * 1. Relative to the note's folder, the Markdown link reading. + * 2. Relative to the vault root, the wikilink reading. Obsidian resolves + * wikilinks from the root, so a pasted `![[assets/img.png]]` inside a note + * under `Daily Notes/` still finds `assets/img.png`, and this is more precise + * than the basename fallback when several files share a name. (#459) + * 3. A unique basename anywhere in the vault, for links written by hand. + */ +export interface AssetPathRef { + path: string +} + +export function stripQueryAndHash(href: string): string { + return href.split('#')[0]?.split('?')[0] ?? href +} + +export function decodeHrefPath(value: string): string { + const cleaned = stripQueryAndHash(value) + try { + return decodeURIComponent(cleaned) + } catch { + return cleaned + } +} + +export function posixJoin(a: string, b: string): string { + if (!a) return b + if (!b) return a + if (a.endsWith('/')) return `${a}${b}` + return `${a}/${b}` +} + +export function posixNormalize(input: string): string { + const parts = input.split('/') + const out: string[] = [] + for (const part of parts) { + if (!part || part === '.') continue + if (part === '..') { + if (out.length === 0) return '..' + out.pop() + } else { + out.push(part) + } + } + return out.join('/') +} + +export function resolveAssetPathAmong( + assets: ReadonlyArray, + notePath: string, + href: string +): string | null { + const trimmed = href.trim() + if (!trimmed || trimmed.startsWith('#') || trimmed.startsWith('//')) return null + if (/^[a-zA-Z][a-zA-Z\d+.-]*:/.test(trimmed)) return null + + const noteDir = notePath.includes('/') ? notePath.slice(0, notePath.lastIndexOf('/')) : '' + const decodedHref = decodeHrefPath(trimmed) + const isAbsolute = decodedHref.startsWith('/') + let target = isAbsolute + ? decodedHref.replace(/^\/+/, '') + : noteDir + ? posixJoin(noteDir, decodedHref) + : decodedHref + target = posixNormalize(target) + if (target.startsWith('../') || target === '..') return null + + if (assets.some((asset) => asset.path === target)) return target + + if (!isAbsolute && noteDir) { + const rootTarget = posixNormalize(decodedHref) + if ( + rootTarget && + rootTarget !== target && + !rootTarget.startsWith('../') && + rootTarget !== '..' && + assets.some((asset) => asset.path === rootTarget) + ) { + return rootTarget + } + } + + const targetBase = target.split('/').filter(Boolean).pop()?.toLowerCase() + if (!targetBase) return null + + const basenameMatches = assets.filter((asset) => { + const assetBase = asset.path.split('/').filter(Boolean).pop()?.toLowerCase() + return assetBase === targetBase + }) + if (basenameMatches.length === 1) { + return basenameMatches[0]!.path + } + + return null +} diff --git a/packages/app-core/src/lib/cloud-auto-sync.test.ts b/packages/app-core/src/lib/cloud-auto-sync.test.ts index 2d38233e..ef574b34 100644 --- a/packages/app-core/src/lib/cloud-auto-sync.test.ts +++ b/packages/app-core/src/lib/cloud-auto-sync.test.ts @@ -2,6 +2,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { CloudAccountStatus, CloudSyncRunSummary, + CloudSyncWindowHandlers, } from "@zennotes/bridge-contract/cloud-sync"; import type { VaultChangeEvent } from "@shared/ipc"; import { @@ -10,6 +11,7 @@ import { closeCloudConflictReview, connectCloudAccountFromStatusBar, openCloudConflictReview, + registerCloudConflictDraftFlusher, startCloudAutoSync, syncCloudVaultWithStatus, type CloudAutoSyncBridge, @@ -152,6 +154,61 @@ describe("cloud auto sync host wiring", () => { vi.useRealTimers(); }); + it("flushes and locks a sibling window review, then closes it from the host's matching result", async () => { + const host = setup(); + let handlers!: CloudSyncWindowHandlers; + const unsubscribe = vi.fn(); + const runtime = startCloudAutoSync({ + ...host.bridge, + onCloudSyncWindow(next) { handlers = next; return unsubscribe; }, + }, host.environment); + await vi.advanceTimersByTimeAsync(1); + await flushPromises(); + let saved!: () => void; + const unregister = registerCloudConflictDraftFlusher( + () => new Promise((resolve) => { saved = resolve; }), + ); + try { + useCloudSyncStatusStore.setState({ conflictReviewOpen: true }); + let prepared = false; + const preparation = handlers.prepare().then(() => { prepared = true; }); + expect(useCloudSyncStatusStore.getState().syncWindowLocked).toBe(true); + await flushPromises(); + expect(prepared).toBe(false); + saved(); + await preparation; + expect(useCloudSyncStatusStore.getState().syncWindowLocked).toBe(true); + const summary = await host.syncCloudVault(); + handlers.finished(summary, null); + expect(useCloudSyncStatusStore.getState()).toMatchObject({ + syncWindowLocked: false, phase: "ready", conflictReviewOpen: false, + lastSummary: summary, + }); + } finally { unregister(); runtime.stop(); } + expect(unsubscribe).toHaveBeenCalledOnce(); + }); + + it("unlocks a failed sibling sync without dismissing the review", async () => { + const host = setup(); + let handlers!: CloudSyncWindowHandlers; + const runtime = startCloudAutoSync({ + ...host.bridge, + onCloudSyncWindow(next) { handlers = next; return () => {}; }, + }, host.environment); + const unregister = registerCloudConflictDraftFlusher(async () => { + throw new Error("Draft save failed"); + }); + try { + useCloudSyncStatusStore.setState({ conflictReviewOpen: true }); + await expect(handlers.prepare()).rejects.toThrow("Draft save failed"); + expect(useCloudSyncStatusStore.getState().syncWindowLocked).toBe(true); + handlers.finished(null, "Draft save failed"); + expect(useCloudSyncStatusStore.getState()).toMatchObject({ + syncWindowLocked: false, phase: "error", conflictReviewOpen: true, + }); + } finally { unregister(); runtime.stop(); } + }); + it("syncs at startup and debounces syncable vault changes", async () => { const host = setup(); const runtime = startCloudAutoSync(host.bridge, host.environment, { @@ -232,6 +289,46 @@ describe("cloud auto sync host wiring", () => { expect(useCloudSyncStatusStore.getState().lastSyncedAt).not.toBeNull(); }); + it("persists a pending review draft before syncing can clear a converged conflict", async () => { + const host = setup(); + let finishSave!: () => void; + const flush = vi.fn( + () => new Promise((resolve) => { finishSave = resolve; }), + ); + const unregister = registerCloudConflictDraftFlusher(flush); + try { + const run = syncCloudVaultWithStatus(host.bridge); + expect(flush).toHaveBeenCalledOnce(); + expect(useCloudSyncStatusStore.getState().phase).toBe("syncing"); + expect(host.syncCloudVault).not.toHaveBeenCalled(); + finishSave(); + await run; + expect(host.syncCloudVault).toHaveBeenCalledOnce(); + } finally { + unregister(); + } + }); + + it("does not run sync or close the review if saving its draft fails", async () => { + const host = setup(); + useCloudSyncStatusStore.setState({ conflictReviewOpen: true }); + const unregister = registerCloudConflictDraftFlusher(async () => { + throw new Error("Draft could not be saved"); + }); + try { + await expect(syncCloudVaultWithStatus(host.bridge)).rejects.toThrow( + "Draft could not be saved", + ); + expect(host.syncCloudVault).not.toHaveBeenCalled(); + expect(useCloudSyncStatusStore.getState()).toMatchObject({ + phase: "error", + conflictReviewOpen: true, + }); + } finally { + unregister(); + } + }); + it("does not report a quota-conflicted sync as successful", async () => { const host = setup(); host.syncCloudVault.mockResolvedValue({ diff --git a/packages/app-core/src/lib/cloud-auto-sync.ts b/packages/app-core/src/lib/cloud-auto-sync.ts index e9d7f17d..5465d71e 100644 --- a/packages/app-core/src/lib/cloud-auto-sync.ts +++ b/packages/app-core/src/lib/cloud-auto-sync.ts @@ -17,6 +17,7 @@ export type CloudAutoSyncBridge = Pick< | "logoutCloudAccount" | "getCloudVaultLink" | "syncCloudVault" + | "onCloudSyncWindow" | "onVaultChange" | "onCloudAccountChange" >; @@ -55,6 +56,8 @@ interface CloudSyncStatusStore { * status bar, so the command palette and the vim leader open the same * queue the status bar's Review now opens. */ conflictReviewOpen: boolean; + /** Remains locked even if this window's own controller refreshes its status. */ + syncWindowLocked: boolean; } const emptyCloudSyncStatus: CloudSyncStatusStore = { @@ -64,6 +67,7 @@ const emptyCloudSyncStatus: CloudSyncStatusStore = { error: null, lastSummary: null, conflictReviewOpen: false, + syncWindowLocked: false, }; export const useCloudSyncStatusStore = create(() => ({ @@ -86,6 +90,17 @@ type CloudAutoSyncTimings = Pick< >; let installedRuntime: CloudAutoSyncRuntime | null = null; +const conflictDraftFlushers = new Set<() => Promise>(); + +/** A review's edits must reach durable storage before sync can retire it. */ +export function registerCloudConflictDraftFlusher( + flush: () => Promise, +): () => void { + conflictDraftFlushers.add(flush); + return () => { + conflictDraftFlushers.delete(flush); + }; +} export function startCloudAutoSync( bridge: CloudAutoSyncBridge, @@ -141,6 +156,17 @@ export function startCloudAutoSync( const unsubscribeVault = bridge.onVaultChange((event) => { if (isSyncableVaultChange(event)) controller.request("local-change"); }); + const unsubscribeSyncWindow = bridge.onCloudSyncWindow?.({ + async prepare() { + useCloudSyncStatusStore.setState({ syncWindowLocked: true }); + await Promise.all([...conflictDraftFlushers].map((flush) => flush())); + }, + finished(summary, error) { + if (summary) applyCloudSyncSummary(summary); + else if (error) useCloudSyncStatusStore.setState({ phase: "error", error }); + useCloudSyncStatusStore.setState({ syncWindowLocked: false }); + }, + }); const unsubscribeAccount = bridge.onCloudAccountChange((status) => { if (status.state === "connecting") markCloudSyncConnecting(); if (status.state === "disconnected") markCloudSyncDisconnected(); @@ -160,6 +186,7 @@ export function startCloudAutoSync( stop() { controller.stop(); unsubscribeVault(); + unsubscribeSyncWindow?.(); unsubscribeAccount(); unsubscribeOnline(); unsubscribeForeground(); @@ -200,31 +227,11 @@ export async function syncCloudVaultWithStatus( }); try { - const summary = await bridge.syncCloudVault(); - const attention = cloudSyncAttentionMessage(summary); - // An open queue stays open only while it still has something to decide; - // otherwise the flag would reopen it on the next unrelated conflict. - const conflictReviewOpen = - current.conflictReviewOpen && resolvableCloudConflictCount(summary) > 0; - if (attention !== null) { - useCloudSyncStatusStore.setState({ - phase: "attention", - vaultName: nextVaultName, - lastSyncedAt: current.lastSyncedAt, - error: attention, - lastSummary: summary, - conflictReviewOpen, - }); - return summary; + if (conflictDraftFlushers.size > 0) { + await Promise.all([...conflictDraftFlushers].map((flush) => flush())); } - useCloudSyncStatusStore.setState({ - phase: "ready", - vaultName: nextVaultName, - lastSyncedAt: Date.now(), - error: null, - lastSummary: summary, - conflictReviewOpen, - }); + const summary = await bridge.syncCloudVault(); + applyCloudSyncSummary(summary, nextVaultName); return summary; } catch (error) { useCloudSyncStatusStore.setState({ @@ -236,6 +243,20 @@ export async function syncCloudVaultWithStatus( } } +function applyCloudSyncSummary(summary: CloudSyncRunSummary, vaultName?: string | null): void { + const current = useCloudSyncStatusStore.getState(); + const attention = cloudSyncAttentionMessage(summary); + useCloudSyncStatusStore.setState({ + phase: attention === null ? "ready" : "attention", + vaultName: vaultName ?? current.vaultName, + lastSyncedAt: attention === null ? Date.now() : current.lastSyncedAt, + error: attention, + lastSummary: summary, + // Do not reopen a finished review on the next unrelated conflict. + conflictReviewOpen: current.conflictReviewOpen && resolvableCloudConflictCount(summary) > 0, + }); +} + /** Conflicts the queue can actually resolve. Bootstrap conflicts are no longer * emitted by the coordinator, so only the durable pending queue counts. */ export function resolvableCloudConflictCount( diff --git a/packages/app-core/src/lib/cm-wikilink-render.ts b/packages/app-core/src/lib/cm-wikilink-render.ts index 1fc2d69f..f7f1a0f9 100644 --- a/packages/app-core/src/lib/cm-wikilink-render.ts +++ b/packages/app-core/src/lib/cm-wikilink-render.ts @@ -23,6 +23,7 @@ import { useStore } from '../store' import { isSameFileBlockLink, isSameFileHeadingLink, resolveWikilinkTarget } from './wikilinks' import { openDatabaseFromWikilink, openWikilinkTarget } from './wikilink-navigation' import { offerCreateNoteFromLink } from './create-note-from-link' +import { openWikilinkAttachment } from './open-wikilink-attachment' // Same shape as the Preview pipeline (remarkWikilinks). const WIKILINK_RE = /(!?)\[\[([^\]|]+?)(?:\|([^\]]+))?\]\]/g @@ -155,6 +156,8 @@ function openWikilink(target: string): void { // Not a note — maybe a `.base` database; otherwise offer to create the note // (with confirmation) so a link to a not-yet-existing note isn't a dead end. if (openDatabaseFromWikilink(target)) return + // A file in the vault (an embedded image, a PDF) opens in its own tab. (#757) + if (openWikilinkAttachment(target)) return void offerCreateNoteFromLink(target) return } diff --git a/packages/app-core/src/lib/connections-outgoing.test.ts b/packages/app-core/src/lib/connections-outgoing.test.ts new file mode 100644 index 00000000..3746488d --- /dev/null +++ b/packages/app-core/src/lib/connections-outgoing.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, it } from 'vitest' +import { classifyOutgoingWikilinks } from './connections-outgoing' + +// #757: the Connections panel listed an embedded image as an unresolved +// wikilink and offered to create `assets/diagram.png.md` for it. A wikilink +// that resolves to a file in the vault is an attachment, never a missing note. + +const notes = [ + { path: 'inbox/Current.md', title: 'Current', folder: 'inbox' as const }, + { path: 'inbox/Doc.md', title: 'Doc', folder: 'inbox' as const }, + { path: 'trash/Old.md', title: 'Old', folder: 'trash' as const } +] + +const assets = [{ path: 'assets/diagram.png' }, { path: 'Papers/spec.pdf' }] + +function classify(body: string, notePath = 'inbox/Current.md') { + return classifyOutgoingWikilinks({ body, notePath, noteTitle: 'Current', notes, assets }) +} + +describe('classifyOutgoingWikilinks (#757)', () => { + it('lists an embedded image as an attachment, not a missing note', () => { + const out = classify('Look: ![[assets/diagram.png]] and [[Doc]] and [[Nowhere]]') + expect(out.resolved.map((n) => n.path)).toEqual(['inbox/Doc.md']) + expect(out.attachments).toEqual([{ target: 'assets/diagram.png', assetPath: 'assets/diagram.png' }]) + expect(out.missing).toEqual([{ target: 'Nowhere', suggestedPath: '/Nowhere.md' }]) + }) + + it('resolves a vault-root path from a note in a subfolder and a bare basename', () => { + const out = classify('[[assets/diagram.png]] [[spec.pdf]]', 'Daily Notes/2026-09-09.md') + expect(out.attachments.map((a) => a.assetPath)).toEqual(['assets/diagram.png', 'Papers/spec.pdf']) + expect(out.missing).toEqual([]) + }) + + it('lists a file reached by two spellings once', () => { + const out = classify('![[assets/diagram.png]] [[diagram.png]]') + expect(out.attachments.map((a) => a.assetPath)).toEqual(['assets/diagram.png']) + }) + + it('keeps a file-like name no asset answers in the missing bucket', () => { + const out = classify('[[assets/nothing.png]]') + expect(out.attachments).toEqual([]) + expect(out.missing.map((m) => m.target)).toEqual(['assets/nothing.png']) + }) + + it('keeps the note rules: self links, trashed notes and duplicate spellings drop out', () => { + const out = classify('[[Current]] [[Old]] [[Doc]] [[Doc#Heading]] [[doc]]') + expect(out.resolved.map((n) => n.path)).toEqual(['inbox/Doc.md']) + expect(out.attachments).toEqual([]) + expect(out.missing.map((m) => m.target)).toEqual(['Old']) + }) +}) diff --git a/packages/app-core/src/lib/connections-outgoing.ts b/packages/app-core/src/lib/connections-outgoing.ts new file mode 100644 index 00000000..cd6670b4 --- /dev/null +++ b/packages/app-core/src/lib/connections-outgoing.ts @@ -0,0 +1,84 @@ +import type { NoteMeta } from '@shared/ipc' +import { resolveAssetPathAmong, type AssetPathRef } from './asset-path-resolution' +import { extractWikilinkTargets, resolveWikilinkTarget, suggestCreateNotePath } from './wikilinks' + +type NoteRef = Pick + +/** A wikilink that names a file in the vault rather than a note. */ +export interface AttachmentLink { + target: string + /** Vault-relative path of the file the wikilink resolves to. */ + assetPath: string +} + +/** A wikilink no note or file answers; the panel offers to create the note. */ +export interface MissingLink { + target: string + suggestedPath: string +} + +export interface OutgoingWikilinks { + resolved: T[] + attachments: AttachmentLink[] + missing: MissingLink[] +} + +/** + * Sorts a note's `[[wikilinks]]` (embeds included, the scanner reads `![[x]]` + * the same) into the notes they reach, the files they reach, and the targets + * nothing answers. The Connections panel shows each bucket differently, and + * the distinction between the last two is what #757 was about: an embedded + * `![[assets/diagram.png]]` used to land in `missing`, and the panel offered + * to create `assets/diagram.png.md` for it. A wikilink is a file when the + * asset list has a file it resolves to, by the same rules that render the + * embed; a name with a file-like extension that resolves to nothing is still + * missing. + * + * First occurrence decides: a target appears in one bucket at most, a note + * reached by two spellings (`[[Doc]]` and `[[Doc#Heading]]`) is listed once. + */ +export function classifyOutgoingWikilinks(args: { + body: string + notePath: string + noteTitle: string + notes: T[] + assets: ReadonlyArray +}): OutgoingWikilinks { + const { body, notePath, noteTitle, notes, assets } = args + const seenTargets = new Set() + const seenPaths = new Set() + const resolved: T[] = [] + const attachments: AttachmentLink[] = [] + const missing: MissingLink[] = [] + const selfTitle = noteTitle.toLowerCase() + + for (const rawTarget of extractWikilinkTargets(body)) { + const target = rawTarget.trim() + if (!target) continue + const targetKey = target.toLowerCase() + if (seenTargets.has(targetKey)) continue + seenTargets.add(targetKey) + + const note = resolveWikilinkTarget(notes, target) + if (note) { + if (note.folder === 'trash' || note.path === notePath) continue + if (seenPaths.has(note.path)) continue + seenPaths.add(note.path) + resolved.push(note) + continue + } + + const assetPath = resolveAssetPathAmong(assets, notePath, target) + if (assetPath) { + if (seenPaths.has(assetPath)) continue + seenPaths.add(assetPath) + attachments.push({ target, assetPath }) + continue + } + + if (targetKey === selfTitle) continue + missing.push({ target, suggestedPath: suggestCreateNotePath(target) }) + } + + return { resolved, attachments, missing } +} diff --git a/packages/app-core/src/lib/follow-link.test.ts b/packages/app-core/src/lib/follow-link.test.ts index 08865470..2320024d 100644 --- a/packages/app-core/src/lib/follow-link.test.ts +++ b/packages/app-core/src/lib/follow-link.test.ts @@ -13,7 +13,9 @@ const state = vi.hoisted(() => ({ ], setFocusedPanel: vi.fn(), editorViewRef: null, - selectNote: vi.fn() + selectNote: vi.fn(), + assetFiles: [{ path: 'assets/diagram.png' }], + openNoteInTab: vi.fn(() => Promise.resolve()) })) const openWikilinkTarget = vi.hoisted(() => vi.fn(() => new Promise(() => undefined))) @@ -41,3 +43,25 @@ describe('followLinkTarget: same-note anchors (#601)', () => { expect(offerCreateNoteFromLink).not.toHaveBeenCalled() }) }) + +describe('followLinkTarget: wikilinks at vault files (#757)', () => { + it('opens the file in an asset tab instead of offering to create a note named after it', () => { + offerCreateNoteFromLink.mockClear() + state.openNoteInTab.mockClear() + + expect(followLinkTarget('assets/diagram.png')).toBe(true) + + expect(state.openNoteInTab).toHaveBeenCalledWith('zen://asset/assets%2Fdiagram.png') + expect(offerCreateNoteFromLink).not.toHaveBeenCalled() + }) + + it('still offers to create a note for a target no file answers', () => { + offerCreateNoteFromLink.mockClear() + state.openNoteInTab.mockClear() + + expect(followLinkTarget('Nowhere')).toBe(true) + + expect(state.openNoteInTab).not.toHaveBeenCalled() + expect(offerCreateNoteFromLink).toHaveBeenCalledWith('Nowhere') + }) +}) diff --git a/packages/app-core/src/lib/follow-link.ts b/packages/app-core/src/lib/follow-link.ts index 0aa6b8f2..acdd22bb 100644 --- a/packages/app-core/src/lib/follow-link.ts +++ b/packages/app-core/src/lib/follow-link.ts @@ -2,6 +2,7 @@ import { useStore } from '../store' import { offerCreateNoteFromLink } from './create-note-from-link' import { externalFileLink, openExternalFileLink } from './external-file-link' import { externalLinkUrl, resolveInternalNoteHref } from './internal-links' +import { openWikilinkAttachment } from './open-wikilink-attachment' import { resolveWikilinkPath } from './wikilinks' import { openDatabaseFromWikilink, @@ -45,6 +46,10 @@ export function followLinkTarget(target: string): boolean { focusSoon() return true } + // A wikilink at a file in the vault (`[[assets/diagram.png]]`, a PDF) opens + // that file in its own tab. Before #757 it fell through to the create offer + // below and proposed a note named `assets/diagram.png.md`. + if (openWikilinkAttachment(target)) return true // A link to a file outside the vault (`~/…`, `file://…`, an absolute path): // open it with the OS default app instead of treating it as a note. (#424) if (externalFileLink(target)) { diff --git a/packages/app-core/src/lib/help.ts b/packages/app-core/src/lib/help.ts index ddfc7311..1b317528 100644 --- a/packages/app-core/src/lib/help.ts +++ b/packages/app-core/src/lib/help.ts @@ -387,7 +387,7 @@ export const HELP_CORE_CONCEPTS: HelpCard[] = [ { title: 'Reference and connections support research-heavy work', body: - 'Pin a companion note or PDF in the reference pane, then toggle the connections panel to inspect backlinks and unresolved links while you draft. Connections count both `[[wikilinks]]` and standard Markdown links (`[text](Note.md)`), so incoming and outgoing associations show up even if you never use wikilinks.' + 'Pin a companion note or PDF in the reference pane, then toggle the connections panel to inspect backlinks and unresolved links while you draft. Connections count both `[[wikilinks]]` and standard Markdown links (`[text](Note.md)`), so incoming and outgoing associations show up even if you never use wikilinks. A wikilink at a file in the vault, such as an embedded image or a PDF, is listed as a file and opens in its own tab; the panel only offers to create a note for a wikilink that reaches nothing.' }, { title: 'Zen mode removes chrome', diff --git a/packages/app-core/src/lib/local-assets.ts b/packages/app-core/src/lib/local-assets.ts index 61aa0731..44d3cc0c 100644 --- a/packages/app-core/src/lib/local-assets.ts +++ b/packages/app-core/src/lib/local-assets.ts @@ -2,6 +2,7 @@ import { useStore } from '../store' import { externalLinkUrl } from './internal-links' import { openVaultAssetExternally } from './external-file-link' import { isExcalidrawPath, isObsidianExcalidrawPath } from '@shared/excalidraw' +import { resolveAssetPathAmong, stripQueryAndHash } from './asset-path-resolution' const IMAGE_EXTENSIONS = new Set([ '.apng', @@ -19,46 +20,11 @@ const VIDEO_EXTENSIONS = new Set(['.m4v', '.mov', '.mp4', '.ogv', '.webm']) export type LocalAssetKind = 'image' | 'pdf' | 'audio' | 'video' | 'excalidraw' | 'file' -function stripQueryAndHash(href: string): string { - return href.split('#')[0]?.split('?')[0] ?? href -} - export function hrefFragment(href: string): string { const hashIdx = href.indexOf('#') return hashIdx >= 0 ? href.slice(hashIdx) : '' } -function decodeHrefPath(value: string): string { - const cleaned = stripQueryAndHash(value) - try { - return decodeURIComponent(cleaned) - } catch { - return cleaned - } -} - -function posixJoin(a: string, b: string): string { - if (!a) return b - if (!b) return a - if (a.endsWith('/')) return `${a}${b}` - return `${a}/${b}` -} - -function posixNormalize(input: string): string { - const parts = input.split('/') - const out: string[] = [] - for (const part of parts) { - if (!part || part === '.') continue - if (part === '..') { - if (out.length === 0) return '..' - out.pop() - } else { - out.push(part) - } - } - return out.join('/') -} - function assetExtension(href: string): string { const clean = stripQueryAndHash(href) const lastDot = clean.lastIndexOf('.') @@ -109,57 +75,7 @@ export function resolveAssetVaultRelativePath( href: string ): string | null { if (!vaultRoot || !notePath) return null - const trimmed = href.trim() - if (!trimmed || trimmed.startsWith('#') || trimmed.startsWith('//')) return null - if (/^[a-zA-Z][a-zA-Z\d+.-]*:/.test(trimmed)) return null - - const noteDir = notePath.includes('/') ? notePath.slice(0, notePath.lastIndexOf('/')) : '' - const decodedHref = decodeHrefPath(trimmed) - const isAbsolute = decodedHref.startsWith('/') - let target = isAbsolute - ? decodedHref.replace(/^\/+/, '') - : noteDir - ? posixJoin(noteDir, decodedHref) - : decodedHref - target = posixNormalize(target) - if (target.startsWith('../') || target === '..') return null - - const assets = useStore.getState().assetFiles - if (assets.some((asset) => asset.path === target)) return target - - // A wikilink embed (`![[assets/img.png]]`) — and any path written relative - // to the vault root — resolves from the root, not the note's folder, which - // is what Obsidian does with wikilinks. When the note-relative join above - // didn't hit an asset, try the path as vault-root-relative before the fuzzy - // basename search below. This is what makes a pasted `![[assets/img.png]]` - // render from a note in a subfolder (e.g. a daily note under - // `Daily Notes/`), and it's more precise than the basename fallback when - // several files share a name. (#459) - if (!isAbsolute && noteDir) { - const rootTarget = posixNormalize(decodedHref) - if ( - rootTarget && - rootTarget !== target && - !rootTarget.startsWith('../') && - rootTarget !== '..' && - assets.some((asset) => asset.path === rootTarget) - ) { - return rootTarget - } - } - - const targetBase = target.split('/').filter(Boolean).pop()?.toLowerCase() - if (!targetBase) return null - - const basenameMatches = assets.filter((asset) => { - const assetBase = asset.path.split('/').filter(Boolean).pop()?.toLowerCase() - return assetBase === targetBase - }) - if (basenameMatches.length === 1) { - return basenameMatches[0]!.path - } - - return null + return resolveAssetPathAmong(useStore.getState().assetFiles, notePath, href) } function localAssetLabel(href: string, fallback: string): string { diff --git a/packages/app-core/src/lib/open-wikilink-attachment.ts b/packages/app-core/src/lib/open-wikilink-attachment.ts new file mode 100644 index 00000000..4cfc431b --- /dev/null +++ b/packages/app-core/src/lib/open-wikilink-attachment.ts @@ -0,0 +1,22 @@ +import { useStore } from '../store' +import { resolveAssetPathAmong } from './asset-path-resolution' +import { assetTabPath } from './asset-tabs' + +/** + * Open the vault file a wikilink names (`[[assets/diagram.png]]`, a PDF, an + * embedded image) in its own tab. Returns false when the target is not an + * existing file, so the caller continues with its own fallbacks. + * + * Three surfaces follow wikilinks with their own resolution chains: the + * cmd-click and table path (follow-link.ts), the rendered-wikilink click + * (cm-wikilink-render.ts) and the Vim `gd` action (Editor.tsx). Each fell + * through to the create offer for a file target and proposed a note named + * `assets/diagram.png.md`. This is the one step they share. (#757) + */ +export function openWikilinkAttachment(target: string): boolean { + const state = useStore.getState() + const assetPath = resolveAssetPathAmong(state.assetFiles, state.selectedPath ?? '', target) + if (!assetPath) return false + void state.openNoteInTab(assetTabPath(assetPath)) + return true +} diff --git a/packages/app-core/src/store.test.ts b/packages/app-core/src/store.test.ts index 56b21161..aa0d5e06 100644 --- a/packages/app-core/src/store.test.ts +++ b/packages/app-core/src/store.test.ts @@ -177,7 +177,7 @@ describe('tasks cache freshness', () => { expect(useStore.getState().vaultTasks).toEqual(freshTasks) }) - it('isolates tasks from a note until its cloud conflict is resolved', async () => { + it('keeps local tasks visible and refreshable while their cloud conflict is pending', async () => { const conflicted = { ...makeTask('choose this later'), id: 'inbox/Conflict.md#0', @@ -189,7 +189,9 @@ describe('tasks cache freshness', () => { sourcePath: 'inbox/Other.md' } const scanTasks = vi.fn().mockResolvedValue([conflicted, unaffected]) - installZen({ scanTasks }) + const edited = { ...conflicted, content: 'local task edited while waiting' } + const scanTasksForPath = vi.fn().mockResolvedValue([edited]) + installZen({ scanTasks, scanTasksForPath }) const { useStore } = await loadStore() const { useCloudSyncStatusStore } = await import('./lib/cloud-auto-sync') @@ -218,10 +220,15 @@ describe('tasks cache freshness', () => { } }) - expect(useStore.getState().vaultTasks).toEqual([unaffected]) + expect(useStore.getState().vaultTasks).toEqual([conflicted, unaffected]) await useStore.getState().refreshTasks() - expect(useStore.getState().vaultTasks).toEqual([unaffected]) + expect(useStore.getState().vaultTasks).toEqual([conflicted, unaffected]) + await useStore.getState().rescanTasksForPath(conflicted.sourcePath) + expect(useStore.getState().vaultTasks).toEqual([unaffected, edited]) + expect(scanTasksForPath).toHaveBeenCalledWith(conflicted.sourcePath) + const resolved = { ...conflicted, content: 'task from the resolved note' } + scanTasks.mockResolvedValue([resolved, unaffected]) useCloudSyncStatusStore.setState({ lastSummary: { cursor: 3, @@ -234,7 +241,7 @@ describe('tasks cache freshness', () => { } }) await useStore.getState().refreshTasks() - expect(useStore.getState().vaultTasks).toEqual([conflicted, unaffected]) + expect(useStore.getState().vaultTasks).toEqual([resolved, unaffected]) }) }) diff --git a/packages/app-core/src/store.ts b/packages/app-core/src/store.ts index 2f8edfb9..b14bc44a 100644 --- a/packages/app-core/src/store.ts +++ b/packages/app-core/src/store.ts @@ -2735,17 +2735,6 @@ function tasksSurfaceVisible(state: { paneLayout: PaneLayout }): boolean { ) } -let isolatedCloudTaskPaths = new Set( - (useCloudSyncStatusStore.getState().lastSummary?.pending_conflicts ?? []).flatMap( - (conflict) => [conflict.path, conflict.cloud_path].filter((path): path is string => Boolean(path)) - ).map(cloudSyncPathKey) -) - -function withoutPendingCloudConflictTasks(tasks: VaultTask[]): VaultTask[] { - if (isolatedCloudTaskPaths.size === 0) return tasks - return tasks.filter((task) => !isolatedCloudTaskPaths.has(cloudSyncPathKey(task.sourcePath))) -} - /** True when the active pane's active tab is the vault-wide Tags view. */ export function isTagsViewActive(state: { paneLayout: PaneLayout @@ -5446,7 +5435,7 @@ export const useStore = create((set, get) => { set({ tasksLoading: true }) try { const tasks = await window.zen.scanTasks() - set({ vaultTasks: withoutPendingCloudConflictTasks(tasks), tasksLoading: false }) + set({ vaultTasks: tasks, tasksLoading: false }) } catch (err) { console.error('scanTasks failed', err) set({ tasksLoading: false }) @@ -5455,9 +5444,7 @@ export const useStore = create((set, get) => { rescanTasksForPath: async (relPath) => { try { - const fresh = isolatedCloudTaskPaths.has(cloudSyncPathKey(relPath)) - ? [] - : await window.zen.scanTasksForPath(relPath) + const fresh = await window.zen.scanTasksForPath(relPath) set((s) => ({ vaultTasks: s.vaultTasks.filter((t) => t.sourcePath !== relPath).concat(fresh) })) @@ -5868,14 +5855,12 @@ export const useStore = create((set, get) => { folder: target.folder }) set((s) => ({ - // Both rebuilt notes go back through the Cloud filter: a note waiting on - // a conflict decision must stay out of the task surfaces even when an - // edit to another note reindexes it. vaultTasks: [ ...s.vaultTasks.filter( (t) => t.sourcePath !== task.sourcePath && t.sourcePath !== target.path ), - ...withoutPendingCloudConflictTasks([...srcTasks, ...tgtTasks]) + ...srcTasks, + ...tgtTasks ] })) }, @@ -5948,13 +5933,12 @@ export const useStore = create((set, get) => { folder: targetMeta.folder }) set((s) => ({ - // Same filter as the move above: forwarding must not slip a withheld - // note's tasks back into the shared cache. vaultTasks: [ ...s.vaultTasks.filter( (t) => t.sourcePath !== task.sourcePath && t.sourcePath !== targetPath ), - ...withoutPendingCloudConflictTasks([...srcTasks, ...tgtTasks]) + ...srcTasks, + ...tgtTasks ] })) }, @@ -10280,7 +10264,7 @@ export function initOverrides(): void { } } -useCloudSyncStatusStore.subscribe((state) => { +useCloudSyncStatusStore.subscribe((state, previous) => { const nextPaths = new Set( (state.lastSummary?.pending_conflicts ?? []).flatMap((conflict) => [conflict.path, conflict.cloud_path] @@ -10288,18 +10272,13 @@ useCloudSyncStatusStore.subscribe((state) => { .map(cloudSyncPathKey) ) ) - if ( - nextPaths.size === isolatedCloudTaskPaths.size && - [...nextPaths].every((path) => isolatedCloudTaskPaths.has(path)) - ) { - return - } - - const restoredPath = [...isolatedCloudTaskPaths].some((path) => !nextPaths.has(path)) - isolatedCloudTaskPaths = nextPaths - useStore.setState((current) => ({ - vaultTasks: withoutPendingCloudConflictTasks(current.vaultTasks) - })) + const restoredPath = (previous.lastSummary?.pending_conflicts ?? []).some((conflict) => + [conflict.path, conflict.cloud_path].some((path) => + path && !nextPaths.has(cloudSyncPathKey(path)) + ) + ) + // Task views always use the local note. A completed decision can replace + // that note, so refresh even when the filesystem watcher coalesces its write. if (restoredPath && tasksSurfaceVisible(useStore.getState())) { void useStore.getState().refreshTasks() } diff --git a/packages/bridge-contract/package.json b/packages/bridge-contract/package.json index bd250581..82da4273 100644 --- a/packages/bridge-contract/package.json +++ b/packages/bridge-contract/package.json @@ -1,7 +1,7 @@ { "name": "@zennotes/bridge-contract", "private": true, - "version": "2.46.0", + "version": "2.47.0", "type": "module", "exports": { "./bridge": "./src/bridge.ts", diff --git a/packages/bridge-contract/src/bridge.ts b/packages/bridge-contract/src/bridge.ts index 442ae40b..d4735eff 100644 --- a/packages/bridge-contract/src/bridge.ts +++ b/packages/bridge-contract/src/bridge.ts @@ -155,11 +155,14 @@ export interface ZenBridge { unlinkCloudVault(): Promise deleteCloudVault(): Promise syncCloudVault(): Promise + /** Hosts with multiple workspace windows coordinate draft saves before sync. */ + onCloudSyncWindow?(handlers: import('./cloud-sync').CloudSyncWindowHandlers): () => void getCloudBootstrapConflict( conflict: CloudSyncBootstrapConflict ): Promise resolveCloudBootstrapConflict(resolution: CloudSyncBootstrapConflictResolution): Promise - getCloudConflict(conflictId: string): Promise + getCloudConflict(conflictId: string, reviewId?: string): Promise + releaseCloudConflictReview?(conflictId: string, reviewId: string): Promise saveCloudConflictDraft(conflictId: string, draftText: string | null): Promise resolveCloudConflict(resolution: CloudSyncPendingConflictResolution): Promise getCloudSettingsConflict(): Promise diff --git a/packages/bridge-contract/src/cloud-sync.ts b/packages/bridge-contract/src/cloud-sync.ts index 222310b5..f868096f 100644 --- a/packages/bridge-contract/src/cloud-sync.ts +++ b/packages/bridge-contract/src/cloud-sync.ts @@ -566,3 +566,17 @@ export interface CloudSyncRunSummary { /** Possible copies made by the pre-v2.44 conflict strategy. Never auto-deleted. */ legacy_conflict_copies?: Array<{ path: string; original_path: string }>; } +/** Desktop-wide sync handshake. Drafts must be durable before a run starts. */ +export type CloudSyncWindowEvent = + | { phase: 'prepare'; requestId: string } + | { + phase: 'finished' + requestId: string + summary: CloudSyncRunSummary | null + error: string | null + } + +export interface CloudSyncWindowHandlers { + prepare(): Promise + finished(summary: CloudSyncRunSummary | null, error: string | null): void +} diff --git a/packages/bridge-contract/src/ipc.ts b/packages/bridge-contract/src/ipc.ts index ee131754..3d4b18d2 100644 --- a/packages/bridge-contract/src/ipc.ts +++ b/packages/bridge-contract/src/ipc.ts @@ -125,6 +125,9 @@ export const IPC = { CLOUD_VAULT_LINK_DELETE: 'cloud-vault-link:delete', CLOUD_VAULT_DELETE: 'cloud-vault:delete', CLOUD_VAULT_SYNC: 'cloud-vault:sync', + CLOUD_VAULT_SYNC_WINDOW: 'cloud-vault:sync-window', + CLOUD_VAULT_SYNC_WINDOW_ACK: 'cloud-vault:sync-window-ack', + CLOUD_VAULT_CONFLICT_REVIEW_RELEASE: 'cloud-vault:conflict-review-release', CLOUD_VAULT_BOOTSTRAP_CONFLICT_GET: 'cloud-vault-bootstrap-conflict:get', CLOUD_VAULT_BOOTSTRAP_CONFLICT_RESOLVE: 'cloud-vault-bootstrap-conflict:resolve', CLOUD_VAULT_CONFLICT_GET: 'cloud-vault-conflict:get', diff --git a/packages/shared-domain/package.json b/packages/shared-domain/package.json index 00615d04..3e94b010 100644 --- a/packages/shared-domain/package.json +++ b/packages/shared-domain/package.json @@ -1,7 +1,7 @@ { "name": "@zennotes/shared-domain", "private": true, - "version": "2.46.0", + "version": "2.47.0", "type": "module", "exports": { "./*": "./src/*.ts" diff --git a/packages/shared-domain/src/cloud-sync-coordinator.test.ts b/packages/shared-domain/src/cloud-sync-coordinator.test.ts index cd61fcfd..68fad3d0 100644 --- a/packages/shared-domain/src/cloud-sync-coordinator.test.ts +++ b/packages/shared-domain/src/cloud-sync-coordinator.test.ts @@ -18,6 +18,7 @@ import type { CloudSyncIdSource, CloudSyncLocalItem, CloudSyncState, + CloudSyncStoredConflict, CloudSyncTrackedItem } from './cloud-sync-engine' import { @@ -1405,6 +1406,205 @@ function upsert(sequence: number, itemId: string, path: string, data: string): C } } +describe('CloudSyncCoordinator: converged pending conflicts', () => { + const path = 'inbox/Plan.md' + const localText = '- [ ] Meet at 10.\n' + const cloudText = '- [ ] Meet at 11.\n' + + function pendingState(): CloudSyncState & { + pending_conflicts: Record + } { + return { + version: 1, + vault_id: 'vault-1', + cursor: 2, + items: { plan: tracked('plan', path, 2, cloudText) }, + pending_conflicts: { + plan: { + id: 'plan', + item_id: 'plan', + kind: 'content', + sequence: 2, + base: { + path, + revision: 1, + kind: 'text', + content: realContent('agreed') + }, + local: { + path, + revision: null, + kind: 'text', + content: realContent(localText) + }, + cloud: { + path, + revision: 2, + kind: 'text', + content: realContent(cloudText) + } + } + } + } + } + + it.each(['local copy', 'remote update'] as const)( + 'clears a pending conflict after a %s makes both versions identical and resumes later edits', + async (convergence) => { + const fs = memoryFileSystem({ [path]: localText }) + const states = memoryState({ + version: 1, + vault_id: 'vault-1', + cursor: 1, + items: { plan: tracked('plan', path, 1, 'agreed') } + }) + const changes = [upsert(2, 'plan', path, cloudText)] + const server = remote({ + changes, + mutate: (body) => ({ + acknowledged: body.mutations.map((mutation) => ({ + operation_id: mutation.operation_id, + item_id: mutation.item_id, + revision: 4, + sequence: 4 + })), + conflicts: [], + cursor: 4 + }) + }) + const repository = new PortableCloudSyncRepository(fs) + const coordinator = new CloudSyncCoordinator('vault-1', server, repository, states, ids()) + const first = await coordinator.sync() + expect(first.pendingConflicts).toEqual([expect.objectContaining({ id: 'plan' })]) + + if (convergence === 'local copy') fs.files.set(path, cloudText) + else changes.push(upsert(3, 'plan', path, localText)) + + // Resume a persisted conflict, including after an app restart. The local + // copy case must work even when the server has no new change to deliver. + const resumed = new CloudSyncCoordinator('vault-1', server, repository, states, ids()) + const result = await resumed.sync() + const agreedText = convergence === 'local copy' ? cloudText : localText + const agreedRevision = convergence === 'local copy' ? 2 : 3 + + expect(result.pendingConflicts).toEqual([]) + expect(states.current?.pending_conflicts).toEqual({}) + expect(result.state.items.plan).toMatchObject( + tracked('plan', path, agreedRevision, agreedText) + ) + expect(fs.files.get(path)).toBe(agreedText) + expect(server.mutations).toEqual([]) + await expect(resumed.getConflict('plan')).rejects.toThrow('no longer waiting') + + fs.files.set(path, `${agreedText}- [ ] Next task.\n`) + const next = await resumed.sync() + expect(next.pendingConflicts).toEqual([]) + expect(next.pushed).toBe(1) + expect(server.mutations.flatMap((request) => request.mutations)).toEqual([ + expect.objectContaining({ + type: 'upsert', + item_id: 'plan', + path, + base_revision: agreedRevision, + content: expect.objectContaining({ + data: `${agreedText}- [ ] Next task.\n` + }) + }) + ]) + } + ) + + it('preserves a saved merge draft when local and Cloud files independently converge', async () => { + const initial = pendingState() + initial.pending_conflicts.plan.draft_text = '- [ ] Meet at 10:30.\n' + const states = memoryState(initial) + const fs = memoryFileSystem({ [path]: cloudText }) + const server = remote({}) + const coordinator = new CloudSyncCoordinator( + 'vault-1', + server, + new PortableCloudSyncRepository(fs), + states, + ids() + ) + + const result = await coordinator.sync() + + expect(result.pendingConflicts).toEqual([expect.objectContaining({ id: 'plan' })]) + await expect(coordinator.getConflict('plan')).resolves.toMatchObject({ + draft_text: '- [ ] Meet at 10:30.\n', + local: { text: cloudText }, + cloud: { text: cloudText } + }) + expect(fs.files.get(path)).toBe(cloudText) + expect(server.mutations).toEqual([]) + }) + + it.each([ + 'different path', + 'case-only path difference', + 'different kind', + 'different byte length', + 'different hash', + 'missing local file', + 'missing tracked item', + 'different tracked identity', + 'stale Cloud snapshot', + 'path conflict', + 'move conflict', + 'delete conflict', + 'repository path conflict' + ])('keeps a pending conflict requiring review for %s', async (scenario) => { + const initial = pendingState() + const conflict = initial.pending_conflicts.plan + const local: CloudSyncLocalItem = { + path, + kind: 'text', + content: realContent(cloudText) + } + const repository = memoryRepository([local]) + + if (scenario === 'different path') local.path = 'archive/Plan.md' + if (scenario === 'case-only path difference') local.path = 'inbox/plan.md' + if (scenario === 'different kind') local.kind = 'binary' + if (scenario === 'different byte length') local.content.byte_length += 1 + if (scenario === 'different hash') local.content.sha256 = realContent('different').sha256 + if (scenario === 'missing local file') repository.items = [] + if (scenario === 'missing tracked item') initial.items = {} + if (scenario === 'different tracked identity') { + initial.items = { + replacement: tracked('replacement', path, 2, cloudText) + } + } + if (scenario === 'stale Cloud snapshot') initial.items.plan = tracked('plan', path, 3, 'newer') + if (scenario === 'path conflict') { + conflict.kind = 'path' + conflict.paused_paths = ['archive/Plan.md'] + } + if (scenario === 'move conflict') conflict.kind = 'move' + if (scenario === 'delete conflict') { + conflict.kind = 'delete' + conflict.cloud = { path: null, revision: 2, kind: 'text', content: null } + initial.items = {} + } + if (scenario === 'repository path conflict') { + repository.pendingConflictPaths = async () => [path] + } + + const states = memoryState(initial) + const server = remote({}) + const coordinator = new CloudSyncCoordinator('vault-1', server, repository, states, ids()) + const result = await coordinator.sync() + + expect(result.pendingConflicts).toEqual([expect.objectContaining({ id: 'plan' })]) + expect(states.current?.pending_conflicts?.plan).toEqual(conflict) + expect(repository.items).toEqual(scenario === 'missing local file' ? [] : [local]) + expect(server.mutations.flatMap((request) => request.mutations)).not.toContainEqual( + expect.objectContaining({ item_id: 'plan' }) + ) + }) +}) + describe('CloudSyncCoordinator: catching up on a file this device never touched', () => { const path = 'inbox/Plan.md' diff --git a/packages/shared-domain/src/cloud-sync-coordinator.ts b/packages/shared-domain/src/cloud-sync-coordinator.ts index 184ab5de..806222e2 100644 --- a/packages/shared-domain/src/cloud-sync-coordinator.ts +++ b/packages/shared-domain/src/cloud-sync-coordinator.ts @@ -402,8 +402,14 @@ export class CloudSyncCoordinator { localConflicts.push(...initialPull.localConflicts) const localItems = await this.repository.scan() + const repositoryPendingPaths = (await this.repository.pendingConflictPaths?.()) ?? [] + const reconciled = clearConvergedConflicts(state, localItems, repositoryPendingPaths) + if (reconciled !== state) { + state = reconciled + await this.states.save(state) + } const pendingPathKeys = new Set([ - ...((await this.repository.pendingConflictPaths?.()) ?? []).map(cloudSyncPathKey), + ...repositoryPendingPaths.map(cloudSyncPathKey), ...pendingConflictPaths(state).map(cloudSyncPathKey) ]) const mutationState = @@ -1200,6 +1206,56 @@ function pendingConflictPaths(state: CloudSyncState): string[] { ) } +function clearConvergedConflicts( + state: CloudSyncState, + localItems: CloudSyncLocalItem[], + repositoryPendingPaths: string[] +): CloudSyncState { + const blocked = new Set(repositoryPendingPaths.map(cloudSyncPathKey)) + const locals = new Map() + for (const item of localItems) { + const key = cloudSyncPathKey(item.path) + if (locals.has(key)) blocked.add(key) + locals.set(key, item) + } + let next = state + for (const conflict of Object.values(state.pending_conflicts ?? {})) { + const cloud = conflict.cloud + const tracked = state.items[conflict.item_id] + if ( + conflict.kind !== 'content' || + (conflict.paused_paths?.length ?? 0) > 0 || + cloud.path === null || + cloud.path !== conflict.local.path || + cloud.path !== conflict.base.path || + !cloud.content || + !tracked || + tracked.item_id !== conflict.item_id || + tracked.path !== cloud.path || + tracked.revision !== cloud.revision || + tracked.kind !== cloud.kind || + tracked.sha256 !== cloud.content.sha256 || + tracked.byte_length !== cloud.content.byte_length || + blocked.has(cloudSyncPathKey(cloud.path)) + ) continue + + const local = locals.get(cloudSyncPathKey(cloud.path)) + if ( + !local || + local.path !== cloud.path || + local.kind !== cloud.kind || + local.content.sha256 !== cloud.content.sha256 || + local.content.byte_length !== cloud.content.byte_length || + (conflict.draft_text !== undefined && conflict.draft_text !== inlineText(local.content)) + ) continue + + // The change feed already advanced the tracked revision. Agreement only + // removes the pause; it never rewrites either file or sends a new version. + next = withoutConflict(next, conflict.id) + } + return next +} + function withAdditionalPausedPaths( state: CloudSyncState, byPathKey: ReadonlyMap> diff --git a/packages/shared-ui/package.json b/packages/shared-ui/package.json index 1ec9520a..ada5aea9 100644 --- a/packages/shared-ui/package.json +++ b/packages/shared-ui/package.json @@ -1,7 +1,7 @@ { "name": "@zennotes/shared-ui", "private": true, - "version": "2.46.0", + "version": "2.47.0", "type": "module", "exports": { ".": "./src/index.ts"