diff --git a/src/LiveSyncBaseCore.ts b/src/LiveSyncBaseCore.ts index 4e4f7e07..edb925bc 100644 --- a/src/LiveSyncBaseCore.ts +++ b/src/LiveSyncBaseCore.ts @@ -95,8 +95,8 @@ export class LiveSyncBaseCore< for (const addOn of addOns) { this._registerAddOn(addOn); } - // Preserve the former ModuleReplicator lifecycle-handler order: - // host features and add-ons first, then replication, then legacy modules. + // Register host features and add-ons before replication, then bind + // legacy modules so lifecycle handlers observe the required order. useReplicationFeature(this); this.bindModuleFunctions(); } diff --git a/src/apps/cli/commands/centralRemoteAdministration.ts b/src/apps/cli/commands/centralRemoteAdministration.ts index 5fded766..f85c944e 100644 --- a/src/apps/cli/commands/centralRemoteAdministration.ts +++ b/src/apps/cli/commands/centralRemoteAdministration.ts @@ -30,6 +30,10 @@ function detailMessage(detail: unknown): string { return detail instanceof Error ? detail.message : String(detail); } +function assertNeverCentralRemoteAdministrationFailureReason(reason: never): never { + throw new Error(`Unexpected central remote administration failure reason: ${String(reason)}`); +} + function reportMilestoneObservation( standardIo: StandardIo, observation: Extract< @@ -56,7 +60,8 @@ function reportCentralRemoteAdministrationResult( return; } - switch (result.reason) { + const reason = result.reason; + switch (reason) { case CENTRAL_REMOTE_ADMINISTRATION_FAILURE_REASONS.NO_ACTIVE_REPLICATOR: standardIo.writeStderr("[Verification] No active replicator found\n"); return; @@ -65,6 +70,11 @@ function reportCentralRemoteAdministrationResult( `[Verification] Failed to connect to the configured remote: ${detailMessage(result.detail)}\n` ); return; + case CENTRAL_REMOTE_ADMINISTRATION_FAILURE_REASONS.ACTIVE_CONFIGURATION_MISMATCH: + standardIo.writeStderr( + "[Verification] The active remote configuration changed before remote administration could begin.\n" + ); + return; case CENTRAL_REMOTE_ADMINISTRATION_FAILURE_REASONS.MILESTONE_NOT_FOUND: standardIo.writeStderr("[Verification] Milestone document not found on remote.\n"); return; @@ -83,6 +93,8 @@ function reportCentralRemoteAdministrationResult( case CENTRAL_REMOTE_ADMINISTRATION_FAILURE_REASONS.POSTCONDITION_MISMATCH: standardIo.writeStderr("[Verification] The requested remote state was not observed.\n"); return; + default: + return assertNeverCentralRemoteAdministrationFailureReason(reason); } } diff --git a/src/apps/cli/commands/runCommand.unit.spec.ts b/src/apps/cli/commands/runCommand.unit.spec.ts index d504d1b6..1b26d6cb 100644 --- a/src/apps/cli/commands/runCommand.unit.spec.ts +++ b/src/apps/cli/commands/runCommand.unit.spec.ts @@ -786,6 +786,29 @@ describe("runCommand abnormal cases", () => { expect(verificationOutput).not.toContain("CouchDB"); }); + it("reports when the active remote configuration changes before administration begins", async () => { + const core = createCoreMock(); + core.services.replicator.runCentralRemoteAdministration.mockResolvedValueOnce({ + status: CENTRAL_REMOTE_ADMINISTRATION_RESULT_STATUSES.VERIFICATION_FAILED, + reason: CENTRAL_REMOTE_ADMINISTRATION_FAILURE_REASONS.ACTIVE_CONFIGURATION_MISMATCH, + }); + + const result = await runCommand(makeOptions("mark-resolved", []), { + ...context, + core, + }); + + expect(result).toBe(false); + const verificationOutput = core.services.context.standardIo.writeStderr.mock.calls + .map(([chunk]: [string | Uint8Array]) => + typeof chunk === "string" ? chunk : new TextDecoder().decode(chunk) + ) + .join(""); + expect(verificationOutput).toContain( + "[Verification] The active remote configuration changed before remote administration could begin.\n" + ); + }); + it("fails by default when remote administration cannot verify its postcondition", async () => { const core = createCoreMock(); core.services.replicator.runCentralRemoteAdministration.mockResolvedValueOnce({ diff --git a/src/common/centralRemoteAdministration.ts b/src/common/centralRemoteAdministration.ts index 790c96a3..ee3f781b 100644 --- a/src/common/centralRemoteAdministration.ts +++ b/src/common/centralRemoteAdministration.ts @@ -23,14 +23,24 @@ import { type SupportedCapability, } from "@vrtmrz/livesync-commonlib/replication"; +/** + * Central milestone administration shared by the two central providers. + * + * The provider definition selects a reader before mutation. CouchDB then owns + * a fresh verification connection, while Object Storage borrows the active + * Journal client. Local node identity is established before either mutation. + */ const JOURNAL_MILESTONE_PATH = "_00000000-milestone.json"; +/** A provider read result, including failures which settled without a throw. */ type CentralMilestoneReadResult = | { readonly milestone: EntryMilestoneInfo | false | undefined } | { readonly failureReason: CentralRemoteAdministrationFailureReason; readonly detail?: unknown }; +/** A settings-bound postcondition reader prepared before remote mutation. */ type PreparedCentralMilestoneReader = () => Promise; +/** Select and validate the provider-specific reader without performing I/O. */ type CentralMilestoneReaderPreparer = ( replicator: CentralRemoteAdministrationReplicator, setting: RemoteDBSettings @@ -39,7 +49,7 @@ type CentralMilestoneReaderPreparer = ( type CouchDBAdministrationReplicator = CentralRemoteAdministrationReplicator & Pick; -type JournalAdministrationClient = Pick; +type JournalAdministrationClient = Pick; function isCentralRemoteAdministrationReplicator( replicator: ReplicatorInstance @@ -147,6 +157,8 @@ function prepareCouchDBMilestoneReader( requireCouchDBAdministrationOperations(replicator); return async () => { + // This verification connection is fresh and owned by this read. It is + // always closed here rather than retained by the active Replicator. let connection: Awaited>; try { connection = await replicator.connectRemoteCouchDBWithSetting(setting, replicator.isMobile(), true); @@ -186,8 +198,8 @@ function isJournalAdministrationClient(client: unknown): client is JournalAdmini return ( typeof client === "object" && client !== null && - "downloadJson" in client && - typeof client.downloadJson === "function" + "downloadJsonWithResult" in client && + typeof client.downloadJsonWithResult === "function" ); } @@ -200,14 +212,33 @@ function requireJournalAdministrationClient( return replicator.client; } +function assertNeverJournalStorageRead(result: never): never { + throw new Error(`Unexpected Journal storage read result: ${String(result)}`); +} + function prepareObjectStorageMilestoneReader( replicator: CentralRemoteAdministrationReplicator ): PreparedCentralMilestoneReader { + // The Journal client belongs to the active Replicator. This reader borrows + // it for the provider's distinct milestone path and must not dispose it. const client = requireJournalAdministrationClient(replicator); return async () => { try { - return { milestone: await client.downloadJson(JOURNAL_MILESTONE_PATH) }; + const result = await client.downloadJsonWithResult(JOURNAL_MILESTONE_PATH); + switch (result.status) { + case "available": + return { milestone: result.value }; + case "not-found": + return { milestone: undefined }; + case "unavailable": + return { + failureReason: CENTRAL_REMOTE_ADMINISTRATION_FAILURE_REASONS.MILESTONE_READ_FAILED, + detail: result.error, + }; + default: + return assertNeverJournalStorageRead(result); + } } catch (error) { return { failureReason: CENTRAL_REMOTE_ADMINISTRATION_FAILURE_REASONS.MILESTONE_READ_FAILED, diff --git a/src/common/centralRemoteAdministration.unit.spec.ts b/src/common/centralRemoteAdministration.unit.spec.ts index 965509ce..a58d366b 100644 --- a/src/common/centralRemoteAdministration.unit.spec.ts +++ b/src/common/centralRemoteAdministration.unit.spec.ts @@ -130,13 +130,18 @@ describe("central remote administration capabilities", () => { }); it("mutates Object Storage and verifies its milestone postcondition", async () => { - const downloadJson = vi.fn(async () => ({ locked: false, accepted_nodes: ["node-1"] })); + const milestone = { locked: false, accepted_nodes: ["node-1"] }; + const downloadJson = vi.fn(async () => milestone); + const downloadJsonWithResult = vi.fn(async () => ({ + status: "available" as const, + value: milestone, + })); const replicator = { nodeid: "node-1", initializeDatabaseForReplication: vi.fn(async () => true), markRemoteLocked: vi.fn(async () => undefined), markRemoteResolved: vi.fn(async () => undefined), - client: { downloadJson }, + client: { downloadJson, downloadJsonWithResult }, }; const setting = { ...DEFAULT_SETTINGS, remoteType: REMOTE_MINIO }; const capability = OBJECT_STORAGE_CENTRAL_REMOTE_ADMINISTRATION_CAPABILITY; @@ -155,7 +160,63 @@ describe("central remote administration capabilities", () => { }, }); expect(replicator.markRemoteResolved).toHaveBeenCalledWith(setting); - expect(downloadJson).toHaveBeenCalledWith("_00000000-milestone.json"); + expect(downloadJsonWithResult).toHaveBeenCalledWith("_00000000-milestone.json"); + expect(downloadJson).not.toHaveBeenCalled(); + }); + + it("keeps a missing Object Storage milestone as an unverified postcondition", async () => { + const downloadJson = vi.fn(async () => false); + const downloadJsonWithResult = vi.fn(async () => ({ status: "not-found" as const })); + const replicator = { + nodeid: "node-1", + initializeDatabaseForReplication: vi.fn(async () => true), + markRemoteLocked: vi.fn(async () => undefined), + markRemoteResolved: vi.fn(async () => undefined), + client: { downloadJson, downloadJsonWithResult }, + }; + + const result = await OBJECT_STORAGE_CENTRAL_REMOTE_ADMINISTRATION_CAPABILITY.run( + replicator as never, + { ...DEFAULT_SETTINGS, remoteType: REMOTE_MINIO }, + { action: CENTRAL_REMOTE_ADMINISTRATION_ACTIONS.MARK_RESOLVED } + ); + + expect(result).toEqual({ + status: CENTRAL_REMOTE_ADMINISTRATION_RESULT_STATUSES.VERIFICATION_FAILED, + reason: CENTRAL_REMOTE_ADMINISTRATION_FAILURE_REASONS.MILESTONE_NOT_FOUND, + }); + expect(downloadJsonWithResult).toHaveBeenCalledWith("_00000000-milestone.json"); + expect(downloadJson).not.toHaveBeenCalled(); + }); + + it("returns a typed failure with diagnostic detail when Object Storage milestone reading is unavailable", async () => { + const diagnostic = new Error("object storage unavailable"); + const downloadJson = vi.fn(async () => false); + const downloadJsonWithResult = vi.fn(async () => ({ + status: "unavailable" as const, + error: diagnostic, + })); + const replicator = { + nodeid: "node-1", + initializeDatabaseForReplication: vi.fn(async () => true), + markRemoteLocked: vi.fn(async () => undefined), + markRemoteResolved: vi.fn(async () => undefined), + client: { downloadJson, downloadJsonWithResult }, + }; + + const result = await OBJECT_STORAGE_CENTRAL_REMOTE_ADMINISTRATION_CAPABILITY.run( + replicator as never, + { ...DEFAULT_SETTINGS, remoteType: REMOTE_MINIO }, + { action: CENTRAL_REMOTE_ADMINISTRATION_ACTIONS.MARK_RESOLVED } + ); + + expect(result).toEqual({ + status: CENTRAL_REMOTE_ADMINISTRATION_RESULT_STATUSES.VERIFICATION_FAILED, + reason: CENTRAL_REMOTE_ADMINISTRATION_FAILURE_REASONS.MILESTONE_READ_FAILED, + detail: diagnostic, + }); + expect(downloadJsonWithResult).toHaveBeenCalledWith("_00000000-milestone.json"); + expect(downloadJson).not.toHaveBeenCalled(); }); it("rejects an incomplete CouchDB milestone adapter before mutation", async () => { @@ -177,14 +238,14 @@ describe("central remote administration capabilities", () => { expect(markRemoteLocked).not.toHaveBeenCalled(); }); - it("rejects an incomplete Object Storage milestone adapter before mutation", async () => { + it("rejects an Object Storage adapter which only exposes lossy milestone reading", async () => { const markRemoteResolved = vi.fn(async () => undefined); const replicator = { nodeid: "node-1", initializeDatabaseForReplication: vi.fn(async () => true), markRemoteLocked: vi.fn(async () => undefined), markRemoteResolved, - client: {}, + client: { downloadJson: vi.fn(async () => false) }, }; await expect( diff --git a/src/common/replicatorConfigurationIdentity.ts b/src/common/replicatorConfigurationIdentity.ts index 80976066..ff6221d8 100644 --- a/src/common/replicatorConfigurationIdentity.ts +++ b/src/common/replicatorConfigurationIdentity.ts @@ -2,6 +2,12 @@ import type { RemoteDBSettings } from "@vrtmrz/livesync-commonlib/compat/common/ type EndpointProjection = readonly [kind: "url" | "invalid-url", value: string]; +/** + * Compare the effective endpoint rather than inconsequential URI spelling. + * Fragments are not sent, query order is immaterial, and redundant trailing + * slashes do not bind a different adapter. Invalid input is retained verbatim + * and tagged so comparison remains deterministic and fails closed. + */ function projectEndpoint(value: string): EndpointProjection { try { const endpoint = new URL(value); @@ -16,6 +22,11 @@ function projectEndpoint(value: string): EndpointProjection { } } +/** + * Mirror the effective custom-header parser: trim each first name/value pair, + * ignore incomplete lines, and let the last duplicate name win. Sorting the + * resulting entries prevents line order alone from replacing a Replicator. + */ function projectHeaders(value: string): readonly (readonly [name: string, value: string])[] { const headers = new Map(); for (const line of value.split("\n")) { diff --git a/src/common/replicatorProviders.ts b/src/common/replicatorProviders.ts index effba2a7..efd8d589 100644 --- a/src/common/replicatorProviders.ts +++ b/src/common/replicatorProviders.ts @@ -47,6 +47,7 @@ interface OneShotOutcomeReplicator extends ReplicatorInstance { openOneShotReplicationWithOutcome(setting: RemoteDBSettings, showResult: boolean): Promise; } +/** Narrow structurally so the shared adapter does not depend on either concrete provider class. */ function isOneShotOutcomeReplicator(instance: ReplicatorInstance): instance is OneShotOutcomeReplicator { return ( "openOneShotReplicationWithOutcome" in instance && @@ -65,6 +66,8 @@ async function runOneShotWithOutcome( return await instance.openOneShotReplicationWithOutcome(setting, showResult); } +// Manual and unattended wrappers share the provider transfer operation, but +// keep interaction authority and result presentation explicit at this boundary. const couchDBUserInitiatedOneShot: UserInitiatedOneShotRunner = async (instance, setting, request) => { return await runOneShotWithOutcome( instance, @@ -91,7 +94,10 @@ const objectStorageUnattendedOneShot: UnattendedOneShotRunner = async (instance, return await runOneShotWithOutcome(instance, setting, false); }; -/** Build the complete central-remote provider policy for one LiveSync host. */ +/** + * Build the complete, deliberately concrete central-provider matrix for one + * LiveSync host. This closed composition is not a runtime provider registry. + */ export function createCentralReplicatorProviderDefinitions( host: CentralReplicatorProviderHost ): ReplicatorProviderDefinitionMap { diff --git a/src/common/replicatorResources.unit.spec.ts b/src/common/replicatorResources.unit.spec.ts index d58861fa..fa0bba62 100644 --- a/src/common/replicatorResources.unit.spec.ts +++ b/src/common/replicatorResources.unit.spec.ts @@ -1,9 +1,10 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; -import { REMOTE_COUCHDB, REMOTE_MINIO } from "@vrtmrz/livesync-commonlib/compat/common/types"; +import { LOG_LEVEL_NOTICE, REMOTE_COUCHDB, REMOTE_MINIO } from "@vrtmrz/livesync-commonlib/compat/common/types"; import type { ObsidianLiveSyncSettings } from "@vrtmrz/livesync-commonlib/compat/common/types"; import { createNewVaultSettings } from "@vrtmrz/livesync-commonlib/settings"; const mocks = vi.hoisted(() => ({ + logger: vi.fn(), couchDB: [] as Array<{ host: unknown; isMobile: ReturnType; @@ -24,6 +25,11 @@ const mocks = vi.hoisted(() => ({ checkSyncInfo: vi.fn(async () => true), })); +vi.mock("@vrtmrz/livesync-commonlib/compat/common/logger", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, Logger: mocks.logger }; +}); + vi.mock("@vrtmrz/livesync-commonlib/compat/pouchdb/negotiation", () => ({ checkSyncInfo: mocks.checkSyncInfo, })); @@ -87,6 +93,7 @@ describe("replicator probe factories", () => { mocks.couchDB.length = 0; mocks.objectStorage.length = 0; mocks.checkSyncInfo.mockReset().mockResolvedValue(true); + mocks.logger.mockClear(); }); it("binds a CouchDB connection probe to a shallow settings snapshot and closes its owned connection", async () => { @@ -129,6 +136,52 @@ describe("replicator probe factories", () => { expect(replicator.getRemoteStatus).toHaveBeenCalledWith(snapshot); }); + it("emits a result Notice only for an explicitly visible successful CouchDB probe", async () => { + const probe = await createCouchDBConnectionProbeFactory({} as never)(createSettings()); + const replicator = mocks.couchDB[0]; + replicator.connectRemoteCouchDBWithSetting.mockResolvedValue({ + info: { db_name: "vault" }, + close: vi.fn(async () => undefined), + }); + + await expect(probe.check({ showResult: true })).resolves.toEqual({ ok: true }); + + expect(mocks.logger).toHaveBeenCalledTimes(1); + expect(mocks.logger).toHaveBeenCalledWith("Connected to vault successfully", LOG_LEVEL_NOTICE); + + mocks.logger.mockClear(); + await expect(probe.check()).resolves.toEqual({ ok: true }); + expect(mocks.logger).not.toHaveBeenCalled(); + }); + + it("emits a result Notice only for an explicitly visible CouchDB connection failure", async () => { + const reason = "connection failed"; + const translatedFailure = "translated CouchDB connection failure"; + const translate = vi.fn(() => translatedFailure); + const settings = createSettings(); + const probe = await createCouchDBConnectionProbeFactory({ services: { context: { translate } } } as never)( + settings + ); + const replicator = mocks.couchDB[0]; + replicator.connectRemoteCouchDBWithSetting.mockResolvedValue(reason); + + await expect(probe.check({ showResult: true })).resolves.toEqual({ ok: false, reason }); + + expect(mocks.logger).toHaveBeenCalledTimes(1); + expect(translate).toHaveBeenCalledWith("liveSyncReplicator.couldNotConnectTo", { + uri: settings.couchDB_URI, + name: settings.couchDB_DBNAME, + db: reason, + }); + expect(mocks.logger).toHaveBeenCalledWith(translatedFailure, LOG_LEVEL_NOTICE); + + mocks.logger.mockClear(); + translate.mockClear(); + await expect(probe.check()).resolves.toEqual({ ok: false, reason }); + expect(mocks.logger).not.toHaveBeenCalled(); + expect(translate).not.toHaveBeenCalled(); + }); + it("creates an unpublished Object Storage replicator for each probe and normalises connection results", async () => { const host = { name: "host" }; const source = createSettings({ remoteType: REMOTE_MINIO }); @@ -252,6 +305,15 @@ describe("replicator probe factories", () => { expect(replicator.closeReplication).toHaveBeenCalledOnce(); }); + it("preserves a CouchDB connection or setup failure for the settings flow to report", async () => { + const reason = "connection failed"; + const resource = await createCouchDBSynchronisationInformationResourceFactory({} as never)(createSettings()); + const replicator = mocks.couchDB[0]; + replicator.connectRemoteCouchDBWithSetting.mockResolvedValue(reason); + + await expect(resource.check()).rejects.toMatchObject({ message: reason }); + }); + it("closes the owned connection when synchronisation-information verification rejects", async () => { const resource = await createCouchDBSynchronisationInformationResourceFactory({} as never)(createSettings()); const replicator = mocks.couchDB[0]; diff --git a/src/common/replicatorResources/connection.ts b/src/common/replicatorResources/connection.ts index 3b52bede..5494b81e 100644 --- a/src/common/replicatorResources/connection.ts +++ b/src/common/replicatorResources/connection.ts @@ -8,6 +8,7 @@ import { LiveSyncCouchDBReplicator, type LiveSyncCouchDBReplicatorEnv, } from "@vrtmrz/livesync-commonlib/compat/replication/couchdb/LiveSyncReplicator"; +import { LOG_LEVEL_NOTICE, Logger } from "@vrtmrz/livesync-commonlib/compat/common/logger"; import { LiveSyncJournalReplicator } from "@vrtmrz/livesync-commonlib/compat/replication/journal/LiveSyncJournalReplicator"; import { createReplicatorDisposer, snapshotRemoteSettings } from "./shared"; @@ -16,7 +17,8 @@ export type ConnectionResourceHost = LiveSyncCouchDBReplicatorEnv; function createCouchDBConnectionProbe( replicator: LiveSyncCouchDBReplicator, - snapshot: RemoteDBSettings + snapshot: RemoteDBSettings, + host: ConnectionResourceHost ): RemoteConnectionProbe { const dispose = createReplicatorDisposer(replicator); return { @@ -28,9 +30,22 @@ function createCouchDBConnectionProbe( false ); if (typeof connection === "string") { + if (options.showResult) { + Logger( + host.services.context.translate("liveSyncReplicator.couldNotConnectTo", { + uri: snapshot.couchDB_URI, + name: snapshot.couchDB_DBNAME, + db: connection, + }), + LOG_LEVEL_NOTICE + ); + } return { ok: false, reason: connection }; } try { + if (options.showResult) { + Logger(`Connected to ${connection.info.db_name} successfully`, LOG_LEVEL_NOTICE); + } return { ok: true }; } finally { await connection.close(); @@ -64,12 +79,14 @@ function createObjectStorageConnectionProbe( * Build an unpublished CouchDB connection probe for one host. * * The probe owns both its concrete Replicator and each connection it opens. It - * never publishes that Replicator as the active provider instance. + * never publishes that Replicator as the active provider instance. A caller + * may request the established result Notice explicitly; ordinary probes remain + * silent. */ export function createCouchDBConnectionProbeFactory(host: ConnectionResourceHost): ConnectionProbeFactory { return (setting) => { const snapshot = snapshotRemoteSettings(setting); - return Promise.resolve(createCouchDBConnectionProbe(new LiveSyncCouchDBReplicator(host), snapshot)); + return Promise.resolve(createCouchDBConnectionProbe(new LiveSyncCouchDBReplicator(host), snapshot, host)); }; } diff --git a/src/common/replicatorResources/synchronisationInformation.ts b/src/common/replicatorResources/synchronisationInformation.ts index 80b38e57..0e9eea5c 100644 --- a/src/common/replicatorResources/synchronisationInformation.ts +++ b/src/common/replicatorResources/synchronisationInformation.ts @@ -10,7 +10,9 @@ import { createReplicatorDisposer, snapshotRemoteSettings } from "./shared"; * Build an unpublished CouchDB synchronisation-information verifier. * * The resource owns its concrete Replicator and connection, and cannot replace - * the active provider instance. + * the active provider instance. Its check resolves to `false` only for observed + * incompatibility; connection, setup, and verification failures reject so the + * caller can report an operational failure separately. */ export function createCouchDBSynchronisationInformationResourceFactory( host: LiveSyncCouchDBReplicatorEnv @@ -26,7 +28,7 @@ export function createCouchDBSynchronisationInformationResourceFactory( true ); if (typeof connection === "string") { - return false; + throw new Error(connection); } try { return await checkSyncInfo(connection.db); diff --git a/src/modules/coreFeatures/ModuleResolveMismatchedTweaks.ts b/src/modules/coreFeatures/ModuleResolveMismatchedTweaks.ts index 6a0121a7..3a8681d3 100644 --- a/src/modules/coreFeatures/ModuleResolveMismatchedTweaks.ts +++ b/src/modules/coreFeatures/ModuleResolveMismatchedTweaks.ts @@ -21,7 +21,23 @@ import type { InjectableServiceHub } from "@vrtmrz/livesync-commonlib/compat/ser import type { LiveSyncCore } from "@/main.ts"; import { REMOTE_P2P } from "@vrtmrz/livesync-commonlib/compat/common/models/setting.const"; import { withOwnedRemoteResource } from "@/common/ownedRemoteResource"; -import { REMOTE_RESOURCE_KINDS } from "@vrtmrz/livesync-commonlib/replication"; +import { + CENTRAL_COMPATIBILITY_REJECTION_REASONS, + REMOTE_RESOURCE_KINDS, + type ReplicationAttemptFailure, + type ReplicatorInstance, +} from "@vrtmrz/livesync-commonlib/replication"; + +interface PreferredRemoteTweakWriter extends ReplicatorInstance { + setPreferredRemoteTweakSettings(setting: ObsidianLiveSyncSettings): Promise; +} + +function canSetPreferredRemoteTweakSettings(replicator: ReplicatorInstance): replicator is PreferredRemoteTweakWriter { + return ( + "setPreferredRemoteTweakSettings" in replicator && + typeof replicator.setPreferredRemoteTweakSettings === "function" + ); +} /** * Localised counterpart of Commonlib's `confName()`, which takes no translator. @@ -114,11 +130,27 @@ export class ModuleResolvingMismatchedTweaks extends AbstractModule { }); } - async _anyAfterConnectCheckFailed(): Promise { - if (!this.core.replicator.tweakSettingsMismatched && !this.core.replicator.preferredTweakValue) return false; - const preferred = this.core.replicator.preferredTweakValue; - if (!preferred) return false; - const ret = await this.services.tweakValue.askResolvingMismatched(preferred); + async _anyAfterConnectCheckFailed(failure: ReplicationAttemptFailure): Promise { + const recovery = failure.outcome.recoveryHint; + if ( + recovery?.reason !== CENTRAL_COMPATIBILITY_REJECTION_REASONS.TWEAK_MISMATCH || + !recovery.preferredTweakValue + ) { + return false; + } + const ret = await this.services.tweakValue.askResolvingMismatched( + { ...recovery.preferredTweakValue }, + async (setting) => { + let updated = false; + await this.services.replicator.runWithActiveReplicatorContext(async (activeContext) => { + if (activeContext !== failure.context) return; + if (!canSetPreferredRemoteTweakSettings(activeContext.replicator)) return; + await activeContext.replicator.setPreferredRemoteTweakSettings({ ...setting }); + updated = true; + }); + return updated; + } + ); if (ret == "OK") return false; if (ret == "CHECKAGAIN") return "CHECKAGAIN"; if (ret == "IGNORE") return true; @@ -236,8 +268,7 @@ export class ModuleResolvingMismatchedTweaks extends AbstractModule { preferredSource: TweakValues, updatePreferredRemote?: (setting: ObsidianLiveSyncSettings) => Promise ): Promise<"OK" | "CHECKAGAIN" | "IGNORE"> { - const [conf, rebuildRequired] = - await this.services.tweakValue.checkAndAskResolvingMismatched(preferredSource); + const [conf, rebuildRequired] = await this.services.tweakValue.checkAndAskResolvingMismatched(preferredSource); if (!conf) return "IGNORE"; const updateRemote = async () => { diff --git a/src/modules/coreFeatures/ModuleResolveMismatchedTweaks.unit.spec.ts b/src/modules/coreFeatures/ModuleResolveMismatchedTweaks.unit.spec.ts index a340bf24..c97506a9 100644 --- a/src/modules/coreFeatures/ModuleResolveMismatchedTweaks.unit.spec.ts +++ b/src/modules/coreFeatures/ModuleResolveMismatchedTweaks.unit.spec.ts @@ -7,7 +7,12 @@ import { } from "@vrtmrz/livesync-commonlib/compat/common/types"; import { ModuleResolvingMismatchedTweaks } from "./ModuleResolveMismatchedTweaks"; import { setLang } from "@/common/translation"; -import { REMOTE_RESOURCE_KINDS } from "@vrtmrz/livesync-commonlib/replication"; +import { + CENTRAL_COMPATIBILITY_REJECTION_REASONS, + REMOTE_RESOURCE_KINDS, + USER_INITIATED_REPLICATION_AUTHORITY, + type ReplicationAttemptFailure, +} from "@vrtmrz/livesync-commonlib/replication"; function createModule(settingsOverride: Partial = {}) { const askSelectStringDialogue = vi.fn(async (..._args: unknown[]): Promise => undefined); @@ -56,6 +61,75 @@ function createModule(settingsOverride: Partial = {}) { } describe("ModuleResolvingMismatchedTweaks", () => { + it("uses the failed attempt hint and writes only through that exact active publication", async () => { + const { module, core } = createModule(); + const attemptPreferred = { + ...(DEFAULT_SETTINGS as unknown as TweakValues), + customChunkSize: 60, + }; + const replacementPreferred = { + ...(DEFAULT_SETTINGS as unknown as TweakValues), + customChunkSize: 99, + }; + let updatePreferredRemote: ((setting: typeof core.settings) => Promise) | undefined; + const askResolvingMismatched = vi.fn( + async (_preferred: unknown, update: (setting: typeof core.settings) => Promise) => { + updatePreferredRemote = update; + return "IGNORE" as const; + } + ); + core._services.tweakValue = { askResolvingMismatched }; + core.replicator = { + tweakSettingsMismatched: true, + preferredTweakValue: replacementPreferred, + }; + const failedSetPreferred = vi.fn(async (_setting: typeof core.settings) => undefined); + const replacementSetPreferred = vi.fn(async (_setting: typeof core.settings) => undefined); + const failedContext = { + provider: {}, + replicator: { setPreferredRemoteTweakSettings: failedSetPreferred }, + configurationIdentity: "profile-a", + }; + const replacementContext = { + provider: {}, + replicator: { setPreferredRemoteTweakSettings: replacementSetPreferred }, + configurationIdentity: "profile-b", + }; + let activeContext = failedContext; + core._services.replicator = { + runWithActiveReplicatorContext: vi.fn(async (task: (context: typeof failedContext) => unknown) => + task(activeContext) + ), + }; + const request = { + context: failedContext, + setting: core.settings, + outcome: { + status: "failed" as const, + error: new Error("directional replication failed"), + recoveryHint: { + reason: CENTRAL_COMPATIBILITY_REJECTION_REASONS.TWEAK_MISMATCH, + preferredTweakValue: attemptPreferred, + }, + }, + showMessage: true, + interaction: USER_INITIATED_REPLICATION_AUTHORITY, + } as unknown as ReplicationAttemptFailure; + + await expect(module._anyAfterConnectCheckFailed(request)).resolves.toBe(true); + + expect(askResolvingMismatched).toHaveBeenCalledWith(attemptPreferred, expect.any(Function)); + const effectiveSetting = { ...core.settings, customChunkSize: 64 }; + await expect(updatePreferredRemote?.(effectiveSetting)).resolves.toBe(true); + expect(failedSetPreferred).toHaveBeenCalledWith(effectiveSetting); + expect(failedSetPreferred.mock.calls[0][0]).not.toBe(effectiveSetting); + + activeContext = replacementContext; + await expect(updatePreferredRemote?.({ ...effectiveSetting, customChunkSize: 72 })).resolves.toBe(false); + expect(failedSetPreferred).toHaveBeenCalledOnce(); + expect(replacementSetPreferred).not.toHaveBeenCalled(); + }); + it("returns an unconfigured remote result without a separate connection preflight", async () => { const { module, core } = createModule(); const read = vi.fn(async () => ({ diff --git a/src/modules/features/SettingDialogue/ObsidianLiveSyncSettingTab.ts b/src/modules/features/SettingDialogue/ObsidianLiveSyncSettingTab.ts index ae6b5527..2b7402b5 100644 --- a/src/modules/features/SettingDialogue/ObsidianLiveSyncSettingTab.ts +++ b/src/modules/features/SettingDialogue/ObsidianLiveSyncSettingTab.ts @@ -960,6 +960,8 @@ export class ObsidianLiveSyncSettingTab extends PluginSettingTab { /** * Checks the edited CouchDB passphrase through an owned synchronisation- * information resource. A missing document may be created by the check. + * Incompatibility and operational failure retain their distinct existing + * result messages. */ checkWorkingPassphrase = async (): Promise => { if (this.editingSettings.remoteType == REMOTE_MINIO) return true; @@ -980,6 +982,15 @@ export class ObsidianLiveSyncSettingTab extends PluginSettingTab { Logger($msg("obsidianLiveSyncSettingTab.logPassphraseNotCompatible"), LOG_LEVEL_NOTICE); return false; } + } catch (error) { + const reason = error instanceof Error ? error.message : String(error); + Logger( + $msg("obsidianLiveSyncSettingTab.logCheckPassphraseFailed", { + db: reason, + }), + LOG_LEVEL_NOTICE + ); + return false; } finally { await resource.dispose(); } diff --git a/src/modules/features/SettingDialogue/ObsidianLiveSyncSettingTab.unit.spec.ts b/src/modules/features/SettingDialogue/ObsidianLiveSyncSettingTab.unit.spec.ts index c10d5c56..cb667137 100644 --- a/src/modules/features/SettingDialogue/ObsidianLiveSyncSettingTab.unit.spec.ts +++ b/src/modules/features/SettingDialogue/ObsidianLiveSyncSettingTab.unit.spec.ts @@ -1,10 +1,13 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; -import { DEFAULT_SETTINGS, REMOTE_COUCHDB } from "@vrtmrz/livesync-commonlib/compat/common/types"; +import { DEFAULT_SETTINGS, LOG_LEVEL_NOTICE, REMOTE_COUCHDB } from "@vrtmrz/livesync-commonlib/compat/common/types"; import { REMOTE_RESOURCE_KINDS } from "@vrtmrz/livesync-commonlib/replication"; const settingsInitialisationMocks = vi.hoisted(() => ({ applySettingsWithInitialisationChoice: vi.fn(), })); +const loggerMocks = vi.hoisted(() => ({ + Logger: vi.fn(), +})); vi.mock("@/deps.ts", () => ({ App: class {}, @@ -18,6 +21,14 @@ vi.mock("@/deps.ts", () => ({ requireApiVersion: vi.fn(() => false), })); vi.mock("@/main.ts", () => ({ default: class {} })); +vi.mock("@vrtmrz/livesync-commonlib/compat/common/logger", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, Logger: loggerMocks.Logger }; +}); +vi.mock("@/common/translation", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, $msg: vi.fn(actual.$msg) }; +}); vi.mock("@vrtmrz/livesync-commonlib/compat/common/coreEnvFunctions", () => ({ getLanguage: vi.fn(() => "en"), compatGlobal: { @@ -58,9 +69,12 @@ vi.mock("./PanePatches.ts", () => ({ panePatches: vi.fn() })); vi.mock("./PaneMaintenance.ts", () => ({ paneMaintenance: vi.fn() })); import { ObsidianLiveSyncSettingTab } from "./ObsidianLiveSyncSettingTab"; +import { $msg } from "@/common/translation"; beforeEach(() => { settingsInitialisationMocks.applySettingsWithInitialisationChoice.mockReset(); + loggerMocks.Logger.mockClear(); + vi.mocked($msg).mockClear(); }); describe("ObsidianLiveSyncSettingTab passphrase verification", () => { @@ -120,6 +134,70 @@ describe("ObsidianLiveSyncSettingTab passphrase verification", () => { expect(getNewReplicator).not.toHaveBeenCalled(); }); + + it("reports a CouchDB connection or setup failure with the connection-failure message", async () => { + const failure = new Error("remote unavailable"); + const check = vi.fn(async () => { + throw failure; + }); + const dispose = vi.fn(async () => undefined); + const createRemoteResource = vi.fn(async () => ({ check, dispose })); + const plugin = { + app: {}, + core: { + services: { + replicator: { createRemoteResource }, + }, + }, + }; + const tab = new ObsidianLiveSyncSettingTab({} as never, plugin as never); + Object.assign(tab, { + _editingSettings: { + ...DEFAULT_SETTINGS, + remoteType: REMOTE_COUCHDB, + }, + }); + + await expect(tab.checkWorkingPassphrase()).resolves.toBe(false); + + expect(vi.mocked($msg)).toHaveBeenCalledWith("obsidianLiveSyncSettingTab.logCheckPassphraseFailed", { + db: failure.message, + }); + expect(vi.mocked($msg)).not.toHaveBeenCalledWith("obsidianLiveSyncSettingTab.logPassphraseNotCompatible"); + expect(loggerMocks.Logger).toHaveBeenCalledWith(expect.any(String), LOG_LEVEL_NOTICE); + expect(dispose).toHaveBeenCalledOnce(); + }); + + it("reports an actual synchronisation-information mismatch with the incompatibility message", async () => { + const check = vi.fn(async () => false); + const dispose = vi.fn(async () => undefined); + const createRemoteResource = vi.fn(async () => ({ check, dispose })); + const plugin = { + app: {}, + core: { + services: { + replicator: { createRemoteResource }, + }, + }, + }; + const tab = new ObsidianLiveSyncSettingTab({} as never, plugin as never); + Object.assign(tab, { + _editingSettings: { + ...DEFAULT_SETTINGS, + remoteType: REMOTE_COUCHDB, + }, + }); + + await expect(tab.checkWorkingPassphrase()).resolves.toBe(false); + + expect(vi.mocked($msg)).toHaveBeenCalledWith("obsidianLiveSyncSettingTab.logPassphraseNotCompatible"); + expect(vi.mocked($msg)).not.toHaveBeenCalledWith( + "obsidianLiveSyncSettingTab.logCheckPassphraseFailed", + expect.anything() + ); + expect(loggerMocks.Logger).toHaveBeenCalledWith(expect.any(String), LOG_LEVEL_NOTICE); + expect(dispose).toHaveBeenCalledOnce(); + }); }); describe("ObsidianLiveSyncSettingTab connection testing", () => { diff --git a/src/serviceFeatures/replication/ReplicateResultProcessor.ts b/src/serviceFeatures/replication/ReplicateResultProcessor.ts index 8ea1e006..e2253f9d 100644 --- a/src/serviceFeatures/replication/ReplicateResultProcessor.ts +++ b/src/serviceFeatures/replication/ReplicateResultProcessor.ts @@ -109,7 +109,7 @@ export class ReplicateResultProcessor { public get isSuspended() { return ( this._suspended || - !this.services.appLifecycle.isReady || + !this.services.appLifecycle.isReady() || this.context.currentSettings().suspendParseReplicationResult || this.services.appLifecycle.isSuspended() ); diff --git a/src/serviceFeatures/replication/ReplicateResultProcessor.unit.spec.ts b/src/serviceFeatures/replication/ReplicateResultProcessor.unit.spec.ts index c59e11be..91e83eb9 100644 --- a/src/serviceFeatures/replication/ReplicateResultProcessor.unit.spec.ts +++ b/src/serviceFeatures/replication/ReplicateResultProcessor.unit.spec.ts @@ -20,6 +20,7 @@ function note(id: string): PouchDB.Core.ExistingDocument { } type SetupOptions = { + applicationReady?: boolean; processSynchroniseResult?: (entry: unknown) => Promise; setSnapshot?: (key: string, value: unknown) => Promise; }; @@ -29,9 +30,10 @@ function setup(options: SetupOptions = {}) { const setSnapshot = vi.fn(options.setSnapshot ?? (async () => undefined)); const runBoundedLocalApplicationActivity = vi.fn(async (task: () => Promise) => await task()); const onCloseActiveReplication = vi.fn(async () => true); + const isReady = vi.fn(() => options.applicationReady ?? true); const core = { services: { - appLifecycle: { isReady: true, isSuspended: () => false }, + appLifecycle: { isReady, isSuspended: () => false }, path: { getPath: (entry: { path: string }) => entry.path }, replication: { databaseQueueCount: reactiveSource(0), @@ -65,6 +67,7 @@ function setup(options: SetupOptions = {}) { services: core.services, } as never); return { + isReady, onCloseActiveReplication, processor, processSynchroniseResult, @@ -73,6 +76,13 @@ function setup(options: SetupOptions = {}) { } describe("ReplicateResultProcessor", () => { + it("suspends result application while the application is not ready", () => { + const { isReady, processor } = setup({ applicationReady: false }); + + expect(processor.isSuspended).toBe(true); + expect(isReady).toHaveBeenCalledOnce(); + }); + it("retires active ownership when a newer remote version is observed", async () => { const { onCloseActiveReplication, processor } = setup(); const versionInfo = { diff --git a/src/serviceFeatures/replication/index.ts b/src/serviceFeatures/replication/index.ts index b29da6e4..2c220b25 100644 --- a/src/serviceFeatures/replication/index.ts +++ b/src/serviceFeatures/replication/index.ts @@ -25,8 +25,7 @@ function ownsLocalApplicationActivity(value: object): value is LocalApplicationA * * Registration order is observable for equal-priority handlers. The host must * call this after host serviceFeatures and add-ons are composed, but before - * legacy modules are bound. This preserves the former ModuleReplicator - * lifecycle-handler order without retaining a public module identity. + * legacy modules are bound, so lifecycle handlers observe the required order. */ export function useReplicationFeature( core: LiveSyncBaseCore diff --git a/src/serviceFeatures/replicationScheduling.ts b/src/serviceFeatures/replicationScheduling.ts index 2c598982..8613e036 100644 --- a/src/serviceFeatures/replicationScheduling.ts +++ b/src/serviceFeatures/replicationScheduling.ts @@ -3,6 +3,7 @@ import type { ObsidianLiveSyncSettings } from "@vrtmrz/livesync-commonlib/compat import type { NecessaryServices } from "@vrtmrz/livesync-commonlib/compat/interfaces/ServiceModule"; import { createInstanceLogFunction } from "@vrtmrz/livesync-commonlib/compat/services/lib/logUtils"; import { + CAPABILITY_UNAVAILABLE_REASONS, isReplicationCompleted, NO_INTERACTION, type ContinuousReplicationRequest, @@ -60,7 +61,8 @@ interface ReplicationSchedulingContext { function isCapabilityUnavailable(result: ReplicationOutcome): boolean { return ( result.status === "blocked" && - (result.reason === "capability-not-applicable" || result.reason === "capability-not-implemented") + (result.reason === CAPABILITY_UNAVAILABLE_REASONS.NOT_APPLICABLE || + result.reason === CAPABILITY_UNAVAILABLE_REASONS.NOT_IMPLEMENTED) ); }