diff --git a/src/apps/cli/commands/p2p.ts b/src/apps/cli/commands/p2p.ts index f6d9f0e6..97bd1602 100644 --- a/src/apps/cli/commands/p2p.ts +++ b/src/apps/cli/commands/p2p.ts @@ -17,6 +17,7 @@ function delay(ms: number): Promise { return new Promise((resolve) => compatGlobal.setTimeout(resolve, ms)); } +/** Parse a CLI timeout expressed as a finite, non-negative number of seconds. */ export function parseTimeoutSeconds(value: string, commandName: string): number { const timeoutSec = Number(value); if (!Number.isFinite(timeoutSec) || timeoutSec < 0) { @@ -54,6 +55,7 @@ function getSortedPeers(service: Pick): CLIP2P .sort((a, b) => a.peerId.localeCompare(b.peerId)); } +/** Connect for a bounded discovery interval, return a stable peer ordering, and disconnect. */ export async function collectPeers( core: LiveSyncBaseCore, p2pService: CLIP2PService | undefined, @@ -137,6 +139,7 @@ export function createPeerConnectionStatsPayload( return payload; } +/** Resolve one peer token, complete pull then push, and disconnect on every settlement. */ export async function syncWithPeer( core: LiveSyncBaseCore, p2pService: CLIP2PService | undefined, @@ -167,6 +170,9 @@ export async function syncWithPeer( if (pullResult && "error" in pullResult && pullResult.error) { throw pullResult.error instanceof Error ? pullResult.error : LiveSyncError.fromError(pullResult.error); } + if (!pullResult || pullResult.status !== "completed") { + throw LiveSyncError.fromError("P2P sync failed while pulling from peer"); + } const pushResult = await service.targetedTransfer.requestPushToPeer(targetPeer.peerId); if (!pushResult || pushResult.ok !== true) { const err: unknown = pushResult && "error" in pushResult ? pushResult.error : undefined; @@ -182,6 +188,7 @@ export async function syncWithPeer( } } +/** Connect the headless P2P host and transfer transport ownership to the caller. */ export async function openP2PHost( core: LiveSyncBaseCore, p2pService: CLIP2PService | undefined diff --git a/src/apps/cli/commands/p2p.unit.spec.ts b/src/apps/cli/commands/p2p.unit.spec.ts index 6cee45bc..8e14a99b 100644 --- a/src/apps/cli/commands/p2p.unit.spec.ts +++ b/src/apps/cli/commands/p2p.unit.spec.ts @@ -14,8 +14,8 @@ function createCore() { function createP2PService() { const connect = vi.fn(async () => undefined); const disconnect = vi.fn(async () => undefined); - const pullFromPeer = vi.fn(async () => ({ ok: true })); - const requestPushToPeer = vi.fn(async () => ({ ok: true })); + const pullFromPeer = vi.fn(async () => ({ status: "completed" as const, ok: true as const })); + const requestPushToPeer = vi.fn(async () => ({ status: "completed" as const, ok: true as const })); return { service: { transportLifecycle: { isConnected: false, connect, disconnect }, @@ -72,6 +72,16 @@ describe("p2p command helpers", () => { expect(requestPushToPeer).toHaveBeenCalledWith("peer-a"); }); + it("rejects a cancelled pull without requesting a peer push", async () => { + const { service, disconnect, pullFromPeer, requestPushToPeer } = createP2PService(); + pullFromPeer.mockResolvedValue({ status: "cancelled" } as never); + + await expect(syncWithPeer(createCore(), service as never, "peer-a", 0)).rejects.toBeDefined(); + + expect(requestPushToPeer).not.toHaveBeenCalled(); + expect(disconnect).toHaveBeenCalledOnce(); + }); + it("preserves the benchmark diagnostics JSONL contract", () => { expect( createPeerConnectionStatsPayload( diff --git a/src/features/P2PSync/P2PReplicator/P2POpenReplicationModal.ts b/src/features/P2PSync/P2PReplicator/P2POpenReplicationModal.ts index b3b527aa..31334ba1 100644 --- a/src/features/P2PSync/P2PReplicator/P2POpenReplicationModal.ts +++ b/src/features/P2PSync/P2PReplicator/P2POpenReplicationModal.ts @@ -3,9 +3,13 @@ import P2POpenReplicationPane from "./P2POpenReplicationPane.svelte"; import { mount, unmount } from "svelte"; import type { P2PServiceViews } from "@vrtmrz/livesync-commonlib/p2p"; +/** + * Reports action completion so the pane does not infer success merely from a + * settled Promise. + */ export type P2POpenReplicationModalCallback = { - onSync: (peerId: string) => Promise; - onSyncAndClose: (peerId: string) => Promise; + onSync: (peerId: string) => Promise; + onSyncAndClose: (peerId: string) => Promise; }; export class P2POpenReplicationModal extends Modal { @@ -35,17 +39,20 @@ export class P2POpenReplicationModal extends Modal { this.rebuildMode = rebuildMode; } - async onSync(peerId: string) { + async onSync(peerId: string): Promise { if (this.callback?.onSync) { - await this.callback.onSync(peerId); + return await this.callback.onSync(peerId); } + return false; } - async onSyncAndClose(peerId: string) { + async onSyncAndClose(peerId: string): Promise { + let completed = false; if (this.callback?.onSyncAndClose) { - await this.callback.onSyncAndClose(peerId); + completed = await this.callback.onSyncAndClose(peerId); } this.close(); + return completed; } override onOpen() { diff --git a/src/features/P2PSync/P2PReplicator/P2POpenReplicationPane.svelte b/src/features/P2PSync/P2PReplicator/P2POpenReplicationPane.svelte index 618c20f9..41e0e11b 100644 --- a/src/features/P2PSync/P2PReplicator/P2POpenReplicationPane.svelte +++ b/src/features/P2PSync/P2PReplicator/P2POpenReplicationPane.svelte @@ -16,8 +16,8 @@ interface Props { p2p: P2PServiceViews; - onSync: (_peerId: string) => Promise; - onSyncAndClose: (_peerId: string) => Promise; + onSync: (_peerId: string) => Promise; + onSyncAndClose: (_peerId: string) => Promise; onClose: () => void; showResult: boolean; rebuildMode?: boolean; @@ -49,8 +49,8 @@ try { syncingPeerId = peerId; Logger(`Starting sync with ${peerId}`, logLevel); - await onSync(peerId); - Logger(`Sync completed with ${peerId}`, logLevel); + const completed = await onSync(peerId); + if (completed) Logger(`Sync completed with ${peerId}`, logLevel); } catch (e) { Logger(`Error during sync: ${e instanceof Error ? e.message : String(e)}`, logLevel); } finally { @@ -61,8 +61,8 @@ try { syncingPeerId = peerId; Logger(`Starting sync with ${peerId}`, logLevel); - await onSyncAndClose(peerId); - Logger(`Sync completed with ${peerId}`, logLevel); + const completed = await onSyncAndClose(peerId); + if (completed) Logger(`Sync completed with ${peerId}`, logLevel); } catch (e) { Logger(`Error during sync: ${e instanceof Error ? e.message : String(e)}`, logLevel); } finally { diff --git a/src/features/P2PSync/P2PReplicator/P2PReplicationUI.ts b/src/features/P2PSync/P2PReplicator/P2PReplicationUI.ts index 65bc7c3e..f5a7859a 100644 --- a/src/features/P2PSync/P2PReplicator/P2PReplicationUI.ts +++ b/src/features/P2PSync/P2PReplicator/P2PReplicationUI.ts @@ -8,7 +8,9 @@ import { P2POpenReplicationModal } from "./P2POpenReplicationModal"; /** * Creates an openReplicationUI factory for Obsidian environments. * Returns a per-replicator closure that opens the P2P Replication modal - * and performs bidirectional sync (pull then push on success). + * and performs bidirectional sync (pull then push on success) through the + * stable targeted-transfer view. The compatibility Replicator argument is + * intentionally unused here and remains available only to the rebuild factory. * * Usage: * const factory = createOpenReplicationUI(app); @@ -17,7 +19,7 @@ import { P2POpenReplicationModal } from "./P2POpenReplicationModal"; export function createOpenReplicationUI( app: App ): (replicator: LiveSyncTrysteroReplicator, p2p: P2PServiceViews) => (showResult: boolean) => Promise { - return (replicator: LiveSyncTrysteroReplicator, p2p: P2PServiceViews) => + return (_replicator: LiveSyncTrysteroReplicator, p2p: P2PServiceViews) => (showResult: boolean): Promise => { const logLevel = showResult ? LOG_LEVEL_NOTICE : LOG_LEVEL_INFO; return new Promise((resolve) => { @@ -37,20 +39,25 @@ export function createOpenReplicationUI( activeSynchronisations++; try { // Pull first, then push only when the pull succeeds. - const pullResult = await replicator.replicateFrom(peerId, showResult); - if (!pullResult?.ok) { + const pullResult = await p2p.targetedTransfer.pullFromPeer(peerId, { + showNotice: showResult, + }); + if (pullResult.status !== "completed" || !pullResult.ok) { sessionResult = false; - return; + return false; } - const pushResult = await replicator.requestSynchroniseToPeer(peerId); - sessionResult = pushResult?.ok ?? true; - if (sessionResult && closeConnection) await replicator.close(); + const pushResult = await p2p.targetedTransfer.requestPushToPeer(peerId); + const completed = pushResult.status === "completed" && pushResult.ok === true; + sessionResult = completed; + if (completed && closeConnection) await p2p.transportLifecycle.disconnect(); + return completed; } catch (e) { Logger( `Error in bidirectional sync with ${peerId}: ${e instanceof Error ? e.message : String(e)}`, logLevel ); sessionResult = false; + return false; } finally { activeSynchronisations--; settleClosedSession(); @@ -114,12 +121,14 @@ export function createOpenRebuildUI( Logger(`Rebuilding from peer ${peerId}`, logLevel); const result = await replicator.replicateFrom(peerId, showResult, true); sessionResult = result?.ok ?? false; + return sessionResult; } catch (e) { Logger( `Error in rebuild from ${peerId}: ${e instanceof Error ? e.message : String(e)}`, logLevel ); sessionResult = false; + return false; } finally { try { replicator.clearOnSetup(); diff --git a/src/features/P2PSync/P2PReplicator/P2PReplicationUI.unit.spec.ts b/src/features/P2PSync/P2PReplicator/P2PReplicationUI.unit.spec.ts index 252e8255..bec605b8 100644 --- a/src/features/P2PSync/P2PReplicator/P2PReplicationUI.unit.spec.ts +++ b/src/features/P2PSync/P2PReplicator/P2PReplicationUI.unit.spec.ts @@ -4,8 +4,8 @@ const modalState = vi.hoisted(() => ({ instances: [] as Array<{ p2p: unknown; callback: { - onSync: (peerId: string) => Promise; - onSyncAndClose: (peerId: string) => Promise; + onSync: (peerId: string) => Promise; + onSyncAndClose: (peerId: string) => Promise; }; onClosed?: () => void; open: ReturnType; @@ -41,8 +41,8 @@ import { createOpenRebuildUI, createOpenReplicationUI } from "./P2PReplicationUI function createReplicator() { return { - replicateFrom: vi.fn(async () => ({ ok: true })), - requestSynchroniseToPeer: vi.fn(async () => ({ ok: true })), + replicateFrom: vi.fn(async () => ({ status: "completed" as const, ok: true as const })), + requestSynchroniseToPeer: vi.fn(async () => ({ status: "completed" as const, ok: true as const })), close: vi.fn(async () => undefined), setOnSetup: vi.fn(), clearOnSetup: vi.fn(), @@ -50,7 +50,16 @@ function createReplicator() { } function createP2PServiceViews() { - return { transportLifecycle: {}, diagnostics: {} } as any; + return { + transportLifecycle: { + disconnect: vi.fn(async () => undefined), + }, + targetedTransfer: { + pullFromPeer: vi.fn(async () => ({ status: "completed" as const, ok: true as const })), + requestPushToPeer: vi.fn(async () => ({ status: "completed" as const, ok: true as const })), + }, + diagnostics: {}, + } as any; } describe("createOpenReplicationUI", () => { @@ -72,35 +81,53 @@ describe("createOpenReplicationUI", () => { it("keeps repeated synchronisation inside the session boundary until the modal closes", async () => { const replicator = createReplicator(); - const session = createOpenReplicationUI({} as any)(replicator, createP2PServiceViews())(true); + const p2p = createP2PServiceViews(); + const session = createOpenReplicationUI({} as any)(replicator, p2p)(true); const modal = modalState.instances[0]; let settled = false; void session.finally(() => { settled = true; }); - await modal.callback.onSync("peer-a"); + await expect(modal.callback.onSync("peer-a")).resolves.toBe(true); await Promise.resolve(); expect(settled).toBe(false); await modal.callback.onSync("peer-b"); - expect(replicator.replicateFrom).toHaveBeenCalledTimes(2); - expect(replicator.requestSynchroniseToPeer).toHaveBeenCalledTimes(2); + expect(p2p.targetedTransfer.pullFromPeer).toHaveBeenCalledTimes(2); + expect(p2p.targetedTransfer.requestPushToPeer).toHaveBeenCalledTimes(2); modal.onClosed?.(); await expect(session).resolves.toBe(true); }); - it("waits for an in-flight synchronisation when the modal closes", async () => { - let finishPull!: (value: { ok: boolean }) => void; + it("routes ordinary peer transfer through the stable targeted-transfer view", async () => { const replicator = createReplicator(); - replicator.replicateFrom.mockImplementation( + const p2p = createP2PServiceViews(); + const session = createOpenReplicationUI({} as any)(replicator, p2p)(true); + const modal = modalState.instances[0]; + + await modal.callback.onSync("peer-a"); + modal.onClosed?.(); + await expect(session).resolves.toBe(true); + + expect(p2p.targetedTransfer.pullFromPeer).toHaveBeenCalledWith("peer-a", { showNotice: true }); + expect(p2p.targetedTransfer.requestPushToPeer).toHaveBeenCalledWith("peer-a"); + expect(replicator.replicateFrom).not.toHaveBeenCalled(); + expect(replicator.requestSynchroniseToPeer).not.toHaveBeenCalled(); + }); + + it("waits for an in-flight synchronisation when the modal closes", async () => { + let finishPull!: (value: { status: "completed"; ok: true }) => void; + const replicator = createReplicator(); + const p2p = createP2PServiceViews(); + p2p.targetedTransfer.pullFromPeer.mockImplementation( async () => - await new Promise<{ ok: boolean }>((resolve) => { + await new Promise<{ status: "completed"; ok: true }>((resolve) => { finishPull = resolve; }) ); - const session = createOpenReplicationUI({} as any)(replicator, createP2PServiceViews())(true); + const session = createOpenReplicationUI({} as any)(replicator, p2p)(true); const modal = modalState.instances[0]; let settled = false; void session.finally(() => { @@ -113,19 +140,21 @@ describe("createOpenReplicationUI", () => { expect(settled).toBe(false); - finishPull({ ok: true }); + finishPull({ status: "completed", ok: true }); await synchronisation; await expect(session).resolves.toBe(true); }); it("closes the P2P connection after a successful sync-and-close action", async () => { const replicator = createReplicator(); - const session = createOpenReplicationUI({} as any)(replicator, createP2PServiceViews())(true); + const p2p = createP2PServiceViews(); + const session = createOpenReplicationUI({} as any)(replicator, p2p)(true); const modal = modalState.instances[0]; await modal.callback.onSyncAndClose("peer-a"); - expect(replicator.close).toHaveBeenCalledOnce(); + expect(p2p.transportLifecycle.disconnect).toHaveBeenCalledOnce(); + expect(replicator.close).not.toHaveBeenCalled(); let settled = false; void session.finally(() => { settled = true; @@ -136,6 +165,20 @@ describe("createOpenReplicationUI", () => { modal.onClosed?.(); await expect(session).resolves.toBe(true); }); + + it("returns a cancelled peer push as non-success to the presentation boundary", async () => { + const replicator = createReplicator(); + const p2p = createP2PServiceViews(); + p2p.targetedTransfer.requestPushToPeer.mockResolvedValue({ status: "cancelled" } as never); + const session = createOpenReplicationUI({} as any)(replicator, p2p)(true); + const modal = modalState.instances[0]; + + const actionResult = await modal.callback.onSync("peer-a"); + modal.onClosed?.(); + + expect(actionResult).toBe(false); + await expect(session).resolves.toBe(false); + }); }); describe("createOpenRebuildUI", () => { @@ -144,11 +187,11 @@ describe("createOpenRebuildUI", () => { }); it("waits for an in-flight rebuild when the modal closes", async () => { - let finishPull!: (value: { ok: boolean }) => void; + let finishPull!: (value: { status: "completed"; ok: true }) => void; const replicator = createReplicator(); replicator.replicateFrom.mockImplementation( async () => - await new Promise<{ ok: boolean }>((resolve) => { + await new Promise<{ status: "completed"; ok: true }>((resolve) => { finishPull = resolve; }) ); @@ -165,8 +208,8 @@ describe("createOpenRebuildUI", () => { expect(settled).toBe(false); - finishPull({ ok: true }); - await rebuild; + finishPull({ status: "completed", ok: true }); + await expect(rebuild).resolves.toBe(true); await expect(session).resolves.toBe(true); expect(replicator.setOnSetup).toHaveBeenCalledOnce(); expect(replicator.replicateFrom).toHaveBeenCalledWith("peer-a", true, true); diff --git a/src/modules/features/SetupWizard/dialogs/SetupRemoteP2P.svelte b/src/modules/features/SetupWizard/dialogs/SetupRemoteP2P.svelte index ec6a55ab..8a79a891 100644 --- a/src/modules/features/SetupWizard/dialogs/SetupRemoteP2P.svelte +++ b/src/modules/features/SetupWizard/dialogs/SetupRemoteP2P.svelte @@ -106,12 +106,12 @@ throw new Error("The P2P Setup connection probe is not available."); } const result = await coordinateP2PSetupConnectionProbe(admission, trialRemoteSetting, async () => { - const map = new Map(); + const map = new Map(); const store = { get: (key: string) => { return Promise.resolve(map.get(key) || null); }, - set: (key: string, value: any) => { + set: (key: string, value: unknown) => { map.set(key, value); return Promise.resolve(); }, @@ -125,7 +125,7 @@ get db() { return Promise.resolve(this); }, - } as SimpleStore; + } as SimpleStore; const dummyPouch = new PouchDB("dummy"); let replicator: TrysteroReplicator | undefined; @@ -134,7 +134,7 @@ events: context.context.events, translate: context.context.translate, settings: trialRemoteSetting, - processReplicatedDocs: async (_docs: any[]) => { + processReplicatedDocs: async (_docs: PouchDB.Core.ExistingDocument[]) => { return; }, confirm: context.services.confirm, diff --git a/src/serviceFeatures/useP2PReplicatorUI.ts b/src/serviceFeatures/useP2PReplicatorUI.ts index 7eb4e51f..ae61ae82 100644 --- a/src/serviceFeatures/useP2PReplicatorUI.ts +++ b/src/serviceFeatures/useP2PReplicatorUI.ts @@ -111,6 +111,8 @@ export function useP2PReplicatorUI( const activeReplicator = replicator.replicator; if (!activeReplicator) return; const settings = host.services.setting.currentSettings(); + // The deprecated compatibility result only opens the ordinary UI; the + // actual transfer runs through the stable targeted-transfer view. void host.services.replicator.runFiniteReplicationActivity( () => activeReplicator.openReplication(settings, false, true, false), { label: "replication" }