From 96ad2737a7e846205e052da371e9a17cf8dfe43e Mon Sep 17 00:00:00 2001 From: vorotamoroz Date: Fri, 23 Oct 2026 13:56:54 +0000 Subject: [PATCH] Adopt Commonlib 0.2.0 with injected device storage --- devs.md | 2 + package-lock.json | 18 +- package.json | 3 +- src/apps/browser/LiveSyncBrowserUIService.ts | 4 +- .../createLiveSyncBrowserServiceHub.ts | 50 +---- ...eateLiveSyncBrowserServiceHub.unit.spec.ts | 43 ++++- src/apps/cli/README.md | 4 +- src/apps/cli/main.bootstrap.unit.spec.ts | 21 ++ src/apps/cli/main.ts | 4 - src/apps/cli/package.json | 2 +- src/apps/cli/services/NodeLocalStorage.ts | 159 +++++++-------- .../services/NodeLocalStorage.unit.spec.ts | 127 +++++++++--- src/apps/cli/services/NodeServiceHub.ts | 19 +- src/apps/cli/services/NodeSettingService.ts | 42 ---- src/apps/webapp/WebAppRuntime.ts | 14 +- src/apps/webpeer/src/WebPeerRuntime.ts | 10 +- src/common/replicatorProviders.ts | 8 + src/common/replicatorProviders.unit.spec.ts | 22 ++- src/common/replicatorResources.unit.spec.ts | 69 +++++++ src/common/replicatorResources/index.ts | 5 + .../replicatorResources/joinedDevices.ts | 40 ++++ src/common/turnSettingsPrivacy.unit.spec.ts | 12 +- .../P2PReplicator/P2PReplicatorPane.svelte | 134 +++++++++++-- .../ReviewHarness/ReviewHarnessView.ts | 3 +- .../ReviewHarness/reviewHarnessController.ts | 27 ++- .../reviewHarnessController.unit.spec.ts | 151 +++++++++++++-- .../features/ModuleObsidianSettingTab.ts | 6 +- .../ModuleObsidianSettingTab.unit.spec.ts | 34 +++- ...iveSyncSettingTab.declarative.unit.spec.ts | 14 +- .../ObsidianLiveSyncSettingTab.ts | 30 ++- .../ObsidianLiveSyncSettingTab.unit.spec.ts | 65 ++++++- .../features/SetupManager.unit.spec.ts | 7 +- .../SetupWizard/dialogs/SetupRemoteP2P.svelte | 44 ++++- src/modules/services/ObsidianServiceHub.ts | 9 +- src/modules/services/ObsidianServices.ts | 7 +- .../services/ObsidianSettingService.ts | 14 -- .../ObsidianSettingService.unit.spec.ts | 39 ++++ src/modules/services/ObsidianUIService.ts | 4 +- src/serviceFeatures/compatibilityReview.ts | 18 +- .../compatibilityReview.unit.spec.ts | 128 +++++++++++- src/serviceFeatures/redFlag.simpleFetch.ts | 37 ++-- .../redFlag.simpleFetch.unit.spec.ts | 84 +++++++- src/serviceFeatures/redFlag.unit.spec.ts | 22 ++- src/serviceFeatures/useReviewHarness.ts | 14 +- .../useReviewHarness.unit.spec.ts | 82 +++++++- test/apps/webapp/WebAppRuntime.unit.spec.ts | 17 +- test/apps/webpeer/WebPeerRuntime.unit.spec.ts | 41 +++- test/contracts/serviceContext.ts | 5 +- test/e2e-obsidian/runner/liveSyncWorkflow.ts | 4 +- test/e2e-obsidian/runner/upgradeWorkflow.ts | 4 +- test/e2e-obsidian/scripts/dialog-mounts.ts | 182 ++++++++++++------ test/e2e-obsidian/scripts/review-harness.ts | 14 +- test/e2e-obsidian/scripts/settings-ui.ts | 26 ++- updates.md | 4 + utils/couchdb/livesync-commonlib.ts | 4 +- utils/flyio/deno.lock | 57 +++++- utils/flyio/generate_setupuri.test.ts | 10 +- utils/livesync-commonlib-version.ts | 2 +- utils/setup/generate_setup_uri.test.ts | 7 +- utils/setup/livesync-commonlib.ts | 12 +- 60 files changed, 1499 insertions(+), 541 deletions(-) delete mode 100644 src/apps/cli/services/NodeSettingService.ts create mode 100644 src/common/replicatorResources/joinedDevices.ts diff --git a/devs.md b/devs.md index a4d3849d..77c31028 100644 --- a/devs.md +++ b/devs.md @@ -309,6 +309,8 @@ Existing legacy Modules continue to register their handlers in `onBindFunction() database and Replicator readiness. See [the declarative settings adapter ADR](docs/adr/2026_08_declarative_settings_adapter.md). - Use `this.services.setting.saveSettingData()` instead of using plugin methods directly +- Inject `@vrtmrz/value-store` at host composition. `setting.deviceValues` uses exact device-local keys; `setting.config` and the Service Hub `config` share the existing Vault and database-suffix scope. Await reads, writes, and deletions before advancing the workflow. Browser and Obsidian hosts retain their Web Storage keys, and the CLI retains its existing flat JSON string map. +- Seed the settings tab with `await settingTab.loadSettings()` before registration. Its editing buffer remains synchronous for Obsidian rendering; storage failures must not replace a previously loaded value with an empty default. ### Database Operations diff --git a/package-lock.json b/package-lock.json index 1d8d4dfb..4cf6a3e2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -23,9 +23,10 @@ "@smithy/types": "^4.14.3", "@smithy/util-retry": "^4.4.5", "@vrtmrz/browser-ui-kit": "0.1.0", - "@vrtmrz/livesync-commonlib": "0.1.44", + "@vrtmrz/livesync-commonlib": "0.2.0", "@vrtmrz/obsidian-plugin-kit": "0.1.4", "@vrtmrz/ui-interactions": "0.1.2", + "@vrtmrz/value-store": "0.1.0", "diff-match-patch": "^1.0.5", "idb": "^8.0.3", "markdown-it": "^14.3.2", @@ -4115,9 +4116,9 @@ } }, "node_modules/@vrtmrz/livesync-commonlib": { - "version": "0.1.44", - "resolved": "https://registry.npmjs.org/@vrtmrz/livesync-commonlib/-/livesync-commonlib-0.1.44.tgz", - "integrity": "sha512-GjXNSLTnWLZzdgrhW3sIOsLqGLP4iMgypPMtcBc2Ju2vMq64TRA1Llfoya1KRIeu6m0G7Xu3ZtkWARTRcHkYYg==", + "version": "0.2.0", + "resolved": "https://registry.npmjs.org/@vrtmrz/livesync-commonlib/-/livesync-commonlib-0.2.0.tgz", + "integrity": "sha512-K24Pkke+FTjZI7DjIV7qkZk+BmC9VebN3da+2cVD9SWOgoGl7OvDyXTHDafbrmmM4vi4ydaydAEh9glO1xHF6A==", "license": "MIT", "dependencies": { "@aws-crypto/sha256-js": "5.2.0", @@ -4128,6 +4129,7 @@ "@smithy/types": "^4.14.3", "@smithy/util-retry": "^4.4.5", "@trystero-p2p/nostr": "0.25.3", + "@vrtmrz/value-store": "0.1.0", "diff-match-patch": "^1.0.5", "events": "^3.3.0", "fflate": "^0.8.2", @@ -4139,9 +4141,11 @@ "pouchdb-adapter-idb": "^9.0.0", "pouchdb-adapter-indexeddb": "^9.0.0", "pouchdb-adapter-memory": "^9.0.0", + "pouchdb-checkpointer": "9.0.0", "pouchdb-core": "^9.0.0", "pouchdb-errors": "^9.0.0", "pouchdb-find": "^9.0.0", + "pouchdb-generate-replication-id": "9.0.0", "pouchdb-mapreduce": "^9.0.0", "pouchdb-merge": "^9.0.0", "pouchdb-replication": "^9.0.0", @@ -4210,6 +4214,12 @@ "integrity": "sha512-njT2BSFh57imwGDxWXJOcGbZOYFVNGmeXMrS7/RUdBoAeOjWzJB9TxI7WcR1sgQIXKh8TE9hqALU8lue5b8dRw==", "license": "MIT" }, + "node_modules/@vrtmrz/value-store": { + "version": "0.1.0", + "resolved": "https://registry.npmjs.org/@vrtmrz/value-store/-/value-store-0.1.0.tgz", + "integrity": "sha512-M62f1j0bnJkqpgI5sYxnkXUAOSDEwQbYrLf+w58gCCm4sjIXDmFzDJd2duXabRbktBlACt9P99yjN/9gOAPxrA==", + "license": "MIT" + }, "node_modules/abstract-leveldown": { "version": "6.2.3", "resolved": "https://registry.npmjs.org/abstract-leveldown/-/abstract-leveldown-6.2.3.tgz", diff --git a/package.json b/package.json index a6ff13d4..f434524a 100644 --- a/package.json +++ b/package.json @@ -193,9 +193,10 @@ "@smithy/types": "^4.14.3", "@smithy/util-retry": "^4.4.5", "@vrtmrz/browser-ui-kit": "0.1.0", - "@vrtmrz/livesync-commonlib": "0.1.44", + "@vrtmrz/livesync-commonlib": "0.2.0", "@vrtmrz/obsidian-plugin-kit": "0.1.4", "@vrtmrz/ui-interactions": "0.1.2", + "@vrtmrz/value-store": "0.1.0", "diff-match-patch": "^1.0.5", "idb": "^8.0.3", "markdown-it": "^14.3.2", diff --git a/src/apps/browser/LiveSyncBrowserUIService.ts b/src/apps/browser/LiveSyncBrowserUIService.ts index ea23cf5a..c0ab9d00 100644 --- a/src/apps/browser/LiveSyncBrowserUIService.ts +++ b/src/apps/browser/LiveSyncBrowserUIService.ts @@ -1,6 +1,6 @@ import type { ServiceContext } from "@vrtmrz/livesync-commonlib/context"; import type { AppLifecycleService } from "@vrtmrz/livesync-commonlib/compat/services/base/AppLifecycleService"; -import type { ConfigService } from "@vrtmrz/livesync-commonlib/compat/services/base/ConfigService"; +import type { IConfigService } from "@vrtmrz/livesync-commonlib/compat/services/base/IService"; import type { ControlService } from "@vrtmrz/livesync-commonlib/compat/services/base/ControlService"; import type { ReplicatorService } from "@vrtmrz/livesync-commonlib/compat/services/base/ReplicatorService"; import type { InjectableAPIService } from "@vrtmrz/livesync-commonlib/compat/services/implements/injectable/InjectableAPIService"; @@ -11,7 +11,7 @@ import { BrowserSvelteDialogManager } from "./BrowserSvelteDialogManager"; export interface LiveSyncBrowserUIServiceDependencies { API: InjectableAPIService; appLifecycle: AppLifecycleService; - config: ConfigService; + config: IConfigService; control: ControlService; replicator: ReplicatorService; } diff --git a/src/apps/browser/createLiveSyncBrowserServiceHub.ts b/src/apps/browser/createLiveSyncBrowserServiceHub.ts index 76c671c0..3197114a 100644 --- a/src/apps/browser/createLiveSyncBrowserServiceHub.ts +++ b/src/apps/browser/createLiveSyncBrowserServiceHub.ts @@ -1,11 +1,10 @@ import type { ObsidianLiveSyncSettings } from "@vrtmrz/livesync-commonlib/compat/common/types"; import type { KeyValueDatabaseFactory } from "@vrtmrz/livesync-commonlib/compat/interfaces/KeyValueDatabase"; import { PouchDB } from "@vrtmrz/livesync-commonlib/compat/pouchdb/pouchdb-browser"; -import { ConfigService } from "@vrtmrz/livesync-commonlib/compat/services/base/ConfigService"; import { ControlService } from "@vrtmrz/livesync-commonlib/compat/services/base/ControlService"; import { DatabaseService } from "@vrtmrz/livesync-commonlib/compat/services/base/DatabaseService"; import { KeyValueDBService } from "@vrtmrz/livesync-commonlib/compat/services/base/KeyValueDBService"; -import type { ISettingService } from "@vrtmrz/livesync-commonlib/compat/services/base/IService"; +import type { ValueDeleter, ValueStore } from "@vrtmrz/value-store"; import { ServiceContext } from "@vrtmrz/livesync-commonlib/context"; import { InjectableAppLifecycleService } from "@vrtmrz/livesync-commonlib/compat/services/implements/injectable/InjectableAppLifecycleService"; import { InjectableConflictService } from "@vrtmrz/livesync-commonlib/compat/services/implements/injectable/InjectableConflictService"; @@ -24,10 +23,7 @@ import { InjectableVaultServiceCompat } from "@vrtmrz/livesync-commonlib/compat/ import { setLang, translateLiveSyncMessage } from "@/common/translation"; import { BrowserConfirm } from "./BrowserConfirm"; import { createBrowserKeyValueDatabaseFactory } from "./BrowserKeyValueDatabase"; -import { - LiveSyncBrowserAPIService, - type LiveSyncBrowserAPIServiceOptions, -} from "./LiveSyncBrowserAPIService"; +import { LiveSyncBrowserAPIService, type LiveSyncBrowserAPIServiceOptions } from "./LiveSyncBrowserAPIService"; import { LiveSyncBrowserUIService } from "./LiveSyncBrowserUIService"; export interface LiveSyncBrowserSettingsPersistence { @@ -43,6 +39,7 @@ export interface LiveSyncBrowserRestartPolicy { } export interface LiveSyncBrowserServiceHubOptions { + deviceValues: ValueStore & ValueDeleter; context?: T; getSystemVaultName?: () => string; settings?: LiveSyncBrowserSettingsPersistence; @@ -51,35 +48,12 @@ export interface LiveSyncBrowserServiceHubOptions { API?: Omit; } -class LiveSyncBrowserAppLifecycleService< - T extends ServiceContext, -> extends InjectableAppLifecycleService {} +class LiveSyncBrowserAppLifecycleService extends InjectableAppLifecycleService {} class LiveSyncBrowserDatabaseService extends DatabaseService {} class LiveSyncBrowserKeyValueDBService extends KeyValueDBService {} -class LiveSyncBrowserConfigService extends ConfigService { - constructor( - context: T, - private readonly setting: ISettingService - ) { - super(context); - } - - getSmallConfig(key: string): string | null { - return this.setting.getSmallConfig(key); - } - - setSmallConfig(key: string, value: string): void { - this.setting.setSmallConfig(key, value); - } - - deleteSmallConfig(key: string): void { - this.setting.deleteSmallConfig(key); - } -} - /** LiveSync-owned service composition shared by WebApp and WebPeer. */ export class LiveSyncBrowserServiceHub extends InjectableServiceHub { constructor(options: LiveSyncBrowserServiceHubOptions) { @@ -98,17 +72,14 @@ export class LiveSyncBrowserServiceHub extends Injecta const setting = new InjectableSettingService(context, { APIService: API, onDisplayLanguageChanged: setLang, + deviceValues: options.deviceValues, }); const settingsPersistence = options.settings; setting.loadData.setHandler( - settingsPersistence - ? () => settingsPersistence.load() - : () => Promise.resolve(undefined) + settingsPersistence ? () => settingsPersistence.load() : () => Promise.resolve(undefined) ); setting.saveData.setHandler( - settingsPersistence - ? (settings) => settingsPersistence.save(settings) - : () => Promise.resolve() + settingsPersistence ? (settings) => settingsPersistence.save(settings) : () => Promise.resolve() ); const appLifecycle = new LiveSyncBrowserAppLifecycleService(context, { @@ -142,7 +113,7 @@ export class LiveSyncBrowserServiceHub extends Injecta setting, API, }); - const config = new LiveSyncBrowserConfigService(context, setting); + const config = setting.config; const replicator = new InjectableReplicatorService(context, { settingService: setting, appLifecycleService: appLifecycle, @@ -163,8 +134,7 @@ export class LiveSyncBrowserServiceHub extends Injecta databaseService: database, }); const keyValueDB = new LiveSyncBrowserKeyValueDBService(context, { - openKeyValueDatabase: - options.openKeyValueDatabase ?? createBrowserKeyValueDatabaseFactory(), + openKeyValueDatabase: options.openKeyValueDatabase ?? createBrowserKeyValueDatabaseFactory(), appLifecycle, databaseEvents, vault, @@ -209,7 +179,7 @@ export class LiveSyncBrowserServiceHub extends Injecta } export function createLiveSyncBrowserServiceHub( - options: LiveSyncBrowserServiceHubOptions = {} + options: LiveSyncBrowserServiceHubOptions ): LiveSyncBrowserServiceHub { return new LiveSyncBrowserServiceHub(options); } diff --git a/src/apps/browser/createLiveSyncBrowserServiceHub.unit.spec.ts b/src/apps/browser/createLiveSyncBrowserServiceHub.unit.spec.ts index 06f90938..558f4fb9 100644 --- a/src/apps/browser/createLiveSyncBrowserServiceHub.unit.spec.ts +++ b/src/apps/browser/createLiveSyncBrowserServiceHub.unit.spec.ts @@ -1,5 +1,6 @@ import { createServiceContext } from "@vrtmrz/livesync-commonlib/context"; import { DEFAULT_SETTINGS } from "@vrtmrz/livesync-commonlib/compat/common/types"; +import { MemoryValueStore } from "@vrtmrz/value-store"; import { afterEach, describe, expect, it, vi } from "vitest"; import { @@ -21,7 +22,8 @@ describe("LiveSync browser service context contract", () => { const context = createServiceContext({ translate: (key) => `webapp:${key}`, }); - const hub = createLiveSyncBrowserServiceHub({ context }); + const deviceValues = new MemoryValueStore(); + const hub = createLiveSyncBrowserServiceHub({ context, deviceValues }); expect(observeServiceContext(context, "moduleLocalDatabase.logWaitingForReady")).toEqual({ translation: "webapp:moduleLocalDatabase.logWaitingForReady", @@ -32,20 +34,17 @@ describe("LiveSync browser service context contract", () => { expect(SERVICE_CONTEXT_MEMBERS.filter((member) => !composition.servicesUsingExpectedContext[member])).toEqual( [] ); + expect(hub.setting.deviceValues).toBe(deviceValues); + expect(hub.config).toBe(hub.setting.config); }); it("binds browser identity, persistence, restart policy, and native Fetch while composing the hub", async () => { - const deviceLocalSettings = new Map(); - vi.stubGlobal("localStorage", { - getItem: (key: string) => deviceLocalSettings.get(key) ?? null, - setItem: (key: string, value: string) => deviceLocalSettings.set(key, value), - removeItem: (key: string) => deviceLocalSettings.delete(key), - clear: () => deviceLocalSettings.clear(), - }); + const deviceValues = new MemoryValueStore(); const savedSettings: (typeof DEFAULT_SETTINGS)[] = []; const restartCalls: string[] = []; const nativeFetch = vi.fn(async () => new Response("ok")); const options: LiveSyncBrowserServiceHubOptions> = { + deviceValues, getSystemVaultName: () => "browser-vault", settings: { load: async () => ({ @@ -69,6 +68,8 @@ describe("LiveSync browser service context contract", () => { const hub = createLiveSyncBrowserServiceHub(options); + expect(hub.setting.deviceValues).toBe(deviceValues); + expect(hub.config).toBe(hub.setting.config); expect(hub.API.getSystemVaultName()).toBe("browser-vault"); expect(hub.API.getPlatform()).toBe("browser"); const response = await hub.API.nativeFetch("https://example.invalid/"); @@ -86,4 +87,30 @@ describe("LiveSync browser service context contract", () => { expect(hub.appLifecycle.isReloadingScheduled()).toBe(true); expect(restartCalls).toEqual(["schedule", "perform", "ask:restart now"]); }); + + it("keeps each browser composition bound to its own device value store", async () => { + const firstValues = new MemoryValueStore(); + const secondValues = new MemoryValueStore(); + const firstHub = createLiveSyncBrowserServiceHub({ + deviceValues: firstValues, + getSystemVaultName: () => "first-vault", + }); + const secondHub = createLiveSyncBrowserServiceHub({ + deviceValues: secondValues, + getSystemVaultName: () => "second-vault", + }); + + await firstHub.config.setValue("probe", "first"); + await secondHub.config.setValue("probe", "second"); + + expect(firstHub.setting.deviceValues).toBe(firstValues); + expect(secondHub.setting.deviceValues).toBe(secondValues); + expect(firstHub.config).toBe(firstHub.setting.config); + expect(secondHub.config).toBe(secondHub.setting.config); + expect(firstHub.config).not.toBe(secondHub.config); + await expect(firstHub.config.getValue("probe")).resolves.toBe("first"); + await expect(secondHub.config.getValue("probe")).resolves.toBe("second"); + await expect(firstValues.getValue("first-vault-probe")).resolves.toBe("first"); + await expect(secondValues.getValue("second-vault-probe")).resolves.toBe("second"); + }); }); diff --git a/src/apps/cli/README.md b/src/apps/cli/README.md index 17674fc3..fb2a7827 100644 --- a/src/apps/cli/README.md +++ b/src/apps/cli/README.md @@ -719,8 +719,8 @@ src/apps/cli/ │ └── ServiceFileAccessImpl.ts ├── services/ │ ├── NodeKeyValueDBService.ts -│ ├── NodeServiceHub.ts -│ └── NodeSettingService.ts +│ ├── NodeLocalStorage.ts +│ └── NodeServiceHub.ts ├── test/ │ ├── test-e2e-two-vaults-common.sh │ ├── test-e2e-two-vaults-matrix.sh diff --git a/src/apps/cli/main.bootstrap.unit.spec.ts b/src/apps/cli/main.bootstrap.unit.spec.ts index 3fcc3913..033771a1 100644 --- a/src/apps/cli/main.bootstrap.unit.spec.ts +++ b/src/apps/cli/main.bootstrap.unit.spec.ts @@ -88,6 +88,27 @@ describe("CLI database preparation", () => { await expect(main(standardIo, runner)).rejects.toThrow(`__EXIT__:${exitCode}`); } + it("loads legacy device values and persists configuration through the service-owned store", async () => { + const localStoragePath = join(directory, ".livesync", "runtime", "local-storage.json"); + await mkdir(join(directory, ".livesync", "runtime"), { recursive: true }); + await writeFile(localStoragePath, JSON.stringify({ ["__proto__"]: "legacy value" })); + let legacyValue: string | null | undefined; + const runner = vi.fn(async (_options, { core }) => { + legacyValue = await core.services.setting.deviceValues.getValue("__proto__"); + expect(core.services.config).toBe(core.services.setting.config); + await core.services.config.setValue("bootstrap-probe", "config value"); + return false; + }); + + await start("ls", runner); + + expect(legacyValue).toBe("legacy value"); + const stored = JSON.parse(await readFile(localStoragePath, "utf-8")) as Record; + expect(Object.prototype.hasOwnProperty.call(stored, "__proto__")).toBe(true); + expect(stored["__proto__"]).toBe("legacy value"); + expect(Object.values(stored)).toContain("config value"); + }); + it.each([REMOTE_P2P, REMOTE_COUCHDB])( "restores configured P2P automation before the host connects (%s)", async (remoteType) => { diff --git a/src/apps/cli/main.ts b/src/apps/cli/main.ts index 028bd465..cea0ec35 100644 --- a/src/apps/cli/main.ts +++ b/src/apps/cli/main.ts @@ -1,6 +1,5 @@ import { useP2PSettingsPreparation } from "@/serviceFeatures/useP2PSettingsPreparation"; import { NodeServiceContext, NodeServiceHub } from "./services/NodeServiceHub"; -import { configureNodeLocalStorage, ensureGlobalNodeLocalStorage } from "./services/NodeLocalStorage"; import { LiveSyncBaseCore, type StartupDatabaseOptions } from "@/LiveSyncBaseCore"; import { initialiseServiceModulesCLI } from "./serviceModules/CLIServiceModules"; import { @@ -78,7 +77,6 @@ const VAULT_SYNC_MODES: Readonly>> }, }; -ensureGlobalNodeLocalStorage(); defaultLoggerEnv.minLogLevel = LOG_LEVEL_DEBUG; /** Injectable command boundary used by CLI integration probes. */ @@ -400,8 +398,6 @@ export async function main( const settingsPath = options.settingsPath ? path.resolve(options.settingsPath) : path.join(databasePath, SETTINGS_FILE); - configureNodeLocalStorage(path.join(databasePath, ".livesync", "runtime", "local-storage.json")); - // Resolve vault path: mirror positional argument takes priority, // then --vault flag, otherwise fall back to databasePath. const vaultPath = diff --git a/src/apps/cli/package.json b/src/apps/cli/package.json index 538c0d99..367e9fce 100644 --- a/src/apps/cli/package.json +++ b/src/apps/cli/package.json @@ -12,7 +12,7 @@ "buildRun": "npm run build && npm run cli --", "build:docker": "docker build -f Dockerfile -t livesync-cli ../../..", "check": "tsc -p tsconfig.json", - "test:unit": "cd ../../.. && npx vitest run --config vitest.config.unit.ts src/apps/cli/main.unit.spec.ts src/apps/cli/main.bootstrap.unit.spec.ts src/apps/cli/settingsPersistence.unit.spec.ts src/apps/cli/commands/utils.unit.spec.ts src/apps/cli/commands/runCommand.unit.spec.ts src/apps/cli/commands/daemonCommand.unit.spec.ts src/apps/cli/commands/p2p.unit.spec.ts src/apps/cli/deploy/install.unit.spec.ts src/apps/cli/adapters/NodeFileSystemAdapter.unit.spec.ts", + "test:unit": "cd ../../.. && npx vitest run --config vitest.config.unit.ts src/apps/cli/main.unit.spec.ts src/apps/cli/main.bootstrap.unit.spec.ts src/apps/cli/settingsPersistence.unit.spec.ts src/apps/cli/services/NodeLocalStorage.unit.spec.ts src/apps/cli/commands/utils.unit.spec.ts src/apps/cli/commands/runCommand.unit.spec.ts src/apps/cli/commands/daemonCommand.unit.spec.ts src/apps/cli/commands/p2p.unit.spec.ts src/apps/cli/deploy/install.unit.spec.ts src/apps/cli/adapters/NodeFileSystemAdapter.unit.spec.ts", "test:e2e:two-vaults": "bash test/test-e2e-two-vaults-with-docker-linux.sh", "test:e2e:two-vaults:common": "bash test/test-e2e-two-vaults-common.sh", "test:e2e:two-vaults:matrix": "bash test/test-e2e-two-vaults-matrix.sh", diff --git a/src/apps/cli/services/NodeLocalStorage.ts b/src/apps/cli/services/NodeLocalStorage.ts index d8149c38..c6997c60 100644 --- a/src/apps/cli/services/NodeLocalStorage.ts +++ b/src/apps/cli/services/NodeLocalStorage.ts @@ -1,115 +1,86 @@ +import { createDeletableValueStore, sanitiseValueStoreError, ValueStoreError } from "@vrtmrz/value-store"; +import type { ValueDeleter, ValueStore } from "@vrtmrz/value-store"; import { fs as nodeFs, path as nodePath } from "@vrtmrz/livesync-commonlib/node"; -import { compatGlobal } from "@vrtmrz/livesync-commonlib/compat/common/coreEnvFunctions"; -type LocalStorageShape = { - getItem(key: string): string | null; - setItem(key: string, value: string): void; - removeItem(key: string): void; - clear(): void; -}; +type NodeLocalStorage = ValueStore & ValueDeleter; -class PersistentNodeLocalStorage { - private storagePath: string | undefined; - private localStore: Record = {}; +class JsonFileValueBackend { + private values: Map; - configure(storagePath: string) { - if (this.storagePath === storagePath) { - return; - } - this.storagePath = storagePath; - this.loadFromFile(); + constructor(private readonly storagePath: string) { + this.values = this.loadValues(); } - private loadFromFile() { - if (!this.storagePath) { - this.localStore = {}; + get(key: string): string | null { + return this.values.get(key) ?? null; + } + + set(key: string, value: string): void { + const nextValues = new Map(this.values); + nextValues.set(key, value); + this.writeValues(nextValues); + this.values = nextValues; + } + + delete(key: string): void { + if (!this.values.has(key)) { return; } + const nextValues = new Map(this.values); + nextValues.delete(key); + this.writeValues(nextValues); + this.values = nextValues; + } + + private loadValues(): Map { + let contents: string; try { - const loaded = JSON.parse(nodeFs.readFileSync(this.storagePath, "utf-8")) as Record; - this.localStore = { ...loaded }; + contents = nodeFs.readFileSync(this.storagePath, "utf-8"); + } catch (error) { + if (isMissingFileError(error)) { + return new Map(); + } + throw sanitiseValueStoreError(error); + } + + let document: unknown; + try { + document = JSON.parse(contents); } catch { - this.localStore = {}; + throw new ValueStoreError("invalid-data"); } + if (document === null || typeof document !== "object" || Array.isArray(document)) { + throw new ValueStoreError("invalid-data"); + } + + const values = new Map(); + for (const [key, value] of Object.entries(document)) { + if (typeof value !== "string") { + throw new ValueStoreError("invalid-data"); + } + values.set(key, value); + } + return values; } - private flushToFile() { - if (!this.storagePath) { - return; - } + private writeValues(values: ReadonlyMap): void { nodeFs.mkdirSync(nodePath.dirname(this.storagePath), { recursive: true }); - nodeFs.writeFileSync(this.storagePath, JSON.stringify(this.localStore, null, 2), "utf-8"); - } - - getItem(key: string): string | null { - return this.localStore[key] ?? null; - } - - setItem(key: string, value: string) { - this.localStore[key] = value; - this.flushToFile(); - } - - removeItem(key: string) { - if (!(key in this.localStore)) { - return; - } - delete this.localStore[key]; - this.flushToFile(); - } - - clear() { - this.localStore = {}; - this.flushToFile(); + nodeFs.writeFileSync(this.storagePath, JSON.stringify(Object.fromEntries(values), null, 2), "utf-8"); } } -const persistentNodeLocalStorage = new PersistentNodeLocalStorage(); - -function createNodeLocalStorageShim(): LocalStorageShape { - return { - getItem(key: string) { - return persistentNodeLocalStorage.getItem(key); - }, - setItem(key: string, value: string) { - persistentNodeLocalStorage.setItem(key, value); - }, - removeItem(key: string) { - persistentNodeLocalStorage.removeItem(key); - }, - clear() { - persistentNodeLocalStorage.clear(); - }, - }; -} - -export function ensureGlobalNodeLocalStorage() { - const g = compatGlobal as unknown as Record; - if ( - !("localStorage" in g) || - typeof (g.localStorage as Record | undefined)?.getItem !== "function" - ) { - g.localStorage = createNodeLocalStorageShim(); +function isMissingFileError(error: unknown): boolean { + if (typeof error !== "object" || error === null) { + return false; + } + try { + const code = Object.getOwnPropertyDescriptor(error, "code"); + return code !== undefined && Object.prototype.hasOwnProperty.call(code, "value") && code.value === "ENOENT"; + } catch { + return false; } } -export function configureNodeLocalStorage(storagePath: string) { - persistentNodeLocalStorage.configure(storagePath); - ensureGlobalNodeLocalStorage(); -} - -export function getNodeLocalStorageItem(key: string): string { - return persistentNodeLocalStorage.getItem(key) ?? ""; -} - -export function setNodeLocalStorageItem(key: string, value: string) { - persistentNodeLocalStorage.setItem(key, value); -} - -export function deleteNodeLocalStorageItem(key: string) { - persistentNodeLocalStorage.removeItem(key); -} - -export function clearNodeLocalStorage() { - persistentNodeLocalStorage.clear(); +export function createNodeLocalStorage(storagePath: string): NodeLocalStorage { + return createDeletableValueStore(new JsonFileValueBackend(storagePath)); } diff --git a/src/apps/cli/services/NodeLocalStorage.unit.spec.ts b/src/apps/cli/services/NodeLocalStorage.unit.spec.ts index fde5aae4..db35b755 100644 --- a/src/apps/cli/services/NodeLocalStorage.unit.spec.ts +++ b/src/apps/cli/services/NodeLocalStorage.unit.spec.ts @@ -1,58 +1,127 @@ import { fs, os, path } from "@vrtmrz/livesync-commonlib/node"; +import { ValueStoreError } from "@vrtmrz/value-store"; import { afterEach, describe, expect, it } from "vitest"; -import { - clearNodeLocalStorage, - configureNodeLocalStorage, - ensureGlobalNodeLocalStorage, - getNodeLocalStorageItem, - setNodeLocalStorageItem, -} from "./NodeLocalStorage"; +import { createNodeLocalStorage } from "./NodeLocalStorage"; describe("NodeLocalStorage", () => { const tempDirs: string[] = []; afterEach(() => { - clearNodeLocalStorage(); for (const tempDir of tempDirs.splice(0)) { fs.rmSync(tempDir, { recursive: true, force: true }); } }); - it("persists values to the configured file", () => { - const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "livesync-node-local-storage-")); - tempDirs.push(tempDir); - const storagePath = path.join(tempDir, "runtime", "local-storage.json"); - - configureNodeLocalStorage(storagePath); - setNodeLocalStorageItem("checkpoint", "42"); - - const saved = JSON.parse(fs.readFileSync(storagePath, "utf-8")) as Record; - expect(saved.checkpoint).toBe("42"); - }); - - it("reloads persisted values when configured again", () => { + it("loads and preserves legacy flat string data, including empty values and exact keys", async () => { const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "livesync-node-local-storage-")); tempDirs.push(tempDir); const storagePath = path.join(tempDir, "runtime", "local-storage.json"); + const specialKey = "__proto__"; + const specialValue = "line one\n雪"; fs.mkdirSync(path.dirname(storagePath), { recursive: true }); - fs.writeFileSync(storagePath, JSON.stringify({ persisted: "value" }, null, 2), "utf-8"); + fs.writeFileSync(storagePath, JSON.stringify({ [specialKey]: specialValue, empty: "" }), "utf-8"); - configureNodeLocalStorage(storagePath); + const store = createNodeLocalStorage(storagePath); - expect(getNodeLocalStorageItem("persisted")).toBe("value"); + await expect(store.getValue(specialKey)).resolves.toBe(specialValue); + await expect(store.getValue("empty", "default")).resolves.toBe(""); + const saved = JSON.parse(fs.readFileSync(storagePath, "utf-8")) as Record; + expect(saved[specialKey]).toBe(specialValue); + expect(saved.empty).toBe(""); }); - it("installs a global localStorage shim backed by the same store", () => { + it("persists strings and objects, reloads them, and deletes keys", async () => { const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "livesync-node-local-storage-")); tempDirs.push(tempDir); const storagePath = path.join(tempDir, "runtime", "local-storage.json"); + const store = createNodeLocalStorage(storagePath); + const object = { enabled: false, entries: ["value", null, 0] }; - configureNodeLocalStorage(storagePath); - ensureGlobalNodeLocalStorage(); + await store.setValue("empty", ""); + await store.setValue("text", "stored value"); + await store.setObject("object", object); - globalThis.localStorage.setItem("shared", "state"); + const reloaded = createNodeLocalStorage(storagePath); + await expect(reloaded.getValue("empty", "default")).resolves.toBe(""); + await expect(reloaded.getValue("text")).resolves.toBe("stored value"); + await expect(reloaded.getObject("object")).resolves.toEqual(object); - expect(getNodeLocalStorageItem("shared")).toBe("state"); + await reloaded.delete("text"); + await expect(reloaded.getValue("text")).resolves.toBeNull(); + const saved = JSON.parse(fs.readFileSync(storagePath, "utf-8")) as Record; + expect(saved.text).toBeUndefined(); + expect(saved.object).toBe(JSON.stringify(object)); + }); + + it("treats a missing file as empty and creates its parent on the first write", async () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "livesync-node-local-storage-")); + tempDirs.push(tempDir); + const storagePath = path.join(tempDir, "runtime", "local-storage.json"); + const store = createNodeLocalStorage(storagePath); + + await expect(store.getValue("absent")).resolves.toBeNull(); + expect(fs.existsSync(storagePath)).toBe(false); + await store.setValue("created", "value"); + expect(fs.existsSync(storagePath)).toBe(true); + }); + + it("keeps separate store instances isolated by their file paths", async () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "livesync-node-local-storage-")); + tempDirs.push(tempDir); + const first = createNodeLocalStorage(path.join(tempDir, "first", "local-storage.json")); + const second = createNodeLocalStorage(path.join(tempDir, "second", "local-storage.json")); + + await first.setValue("shared-key", "first"); + await second.setValue("shared-key", "second"); + + await expect(first.getValue("shared-key")).resolves.toBe("first"); + await expect(second.getValue("shared-key")).resolves.toBe("second"); + }); + + it("surfaces malformed and unreadable files as sanitised value-store errors", async () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "livesync-node-local-storage-")); + tempDirs.push(tempDir); + const malformedPath = path.join(tempDir, "malformed.json"); + fs.writeFileSync(malformedPath, "{ malformed", "utf-8"); + + let malformedError: unknown; + try { + createNodeLocalStorage(malformedPath); + } catch (error) { + malformedError = error; + } + expect(malformedError).toBeInstanceOf(ValueStoreError); + expect(malformedError).toMatchObject({ code: "invalid-data" }); + expect(fs.readFileSync(malformedPath, "utf-8")).toBe("{ malformed"); + + const unreadablePath = path.join(tempDir, "unreadable.json"); + fs.mkdirSync(unreadablePath); + expect(() => createNodeLocalStorage(unreadablePath)).toThrowError(ValueStoreError); + }); + + it("does not change its visible values when a write fails", async () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "livesync-node-local-storage-")); + tempDirs.push(tempDir); + const storagePath = path.join(tempDir, "runtime", "local-storage.json"); + const store = createNodeLocalStorage(storagePath); + await store.setValue("retained", "before failure"); + + fs.rmSync(storagePath); + fs.mkdirSync(storagePath); + + await expect(store.setValue("new", "write fails")).rejects.toBeInstanceOf(ValueStoreError); + await expect(store.getValue("retained")).resolves.toBe("before failure"); + await expect(store.getValue("new")).resolves.toBeNull(); + }); + + it("does not install or modify a global localStorage value", () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "livesync-node-local-storage-")); + tempDirs.push(tempDir); + const before = Object.getOwnPropertyDescriptor(globalThis, "localStorage"); + + createNodeLocalStorage(path.join(tempDir, "runtime", "local-storage.json")); + + expect(Object.getOwnPropertyDescriptor(globalThis, "localStorage")).toEqual(before); }); }); diff --git a/src/apps/cli/services/NodeServiceHub.ts b/src/apps/cli/services/NodeServiceHub.ts index bed9cb16..484861c2 100644 --- a/src/apps/cli/services/NodeServiceHub.ts +++ b/src/apps/cli/services/NodeServiceHub.ts @@ -1,6 +1,5 @@ import type { AppLifecycleServiceDependencies } from "@vrtmrz/livesync-commonlib/compat/services/base/AppLifecycleService"; import type { ServiceContext } from "@vrtmrz/livesync-commonlib/context"; -import { ConfigServiceBrowserCompat } from "@vrtmrz/livesync-commonlib/compat/services/implements/browser/ConfigServiceBrowserCompat"; import type { ComponentHasResult, SvelteDialogManager, @@ -11,6 +10,7 @@ import { InjectableAppLifecycleService } from "@vrtmrz/livesync-commonlib/compat import { InjectableConflictService } from "@vrtmrz/livesync-commonlib/compat/services/implements/injectable/InjectableConflictService"; import { InjectableDatabaseEventService } from "@vrtmrz/livesync-commonlib/compat/services/implements/injectable/InjectableDatabaseEventService"; import { InjectableFileProcessingService } from "@vrtmrz/livesync-commonlib/compat/services/implements/injectable/InjectableFileProcessingService"; +import { InjectableSettingService } from "@vrtmrz/livesync-commonlib/compat/services/implements/injectable/InjectableSettingService"; import { PathServiceCompat } from "@vrtmrz/livesync-commonlib/compat/services/implements/injectable/InjectablePathService"; import { InjectableRemoteService } from "@vrtmrz/livesync-commonlib/compat/services/implements/injectable/InjectableRemoteService"; import { InjectableReplicationService } from "@vrtmrz/livesync-commonlib/compat/services/implements/injectable/InjectableReplicationService"; @@ -21,7 +21,7 @@ import { InjectableVaultServiceCompat } from "@vrtmrz/livesync-commonlib/compat/ import { ControlService } from "@vrtmrz/livesync-commonlib/compat/services/base/ControlService"; import { HeadlessAPIService } from "@vrtmrz/livesync-commonlib/compat/services/implements/headless/HeadlessAPIService"; import { NodeKeyValueDBService } from "./NodeKeyValueDBService"; -import { NodeSettingService } from "./NodeSettingService"; +import { createNodeLocalStorage } from "./NodeLocalStorage"; import { DatabaseService } from "@vrtmrz/livesync-commonlib/compat/services/base/DatabaseService"; import type { ObsidianLiveSyncSettings } from "@vrtmrz/livesync-commonlib/compat/common/types"; import { path as nodePath } from "@vrtmrz/livesync-commonlib/node"; @@ -96,11 +96,11 @@ export class NodeServiceHub extends InjectableServ const conflict = new InjectableConflictService(context); const fileProcessing = new InjectableFileProcessingService(context); - const setting = new NodeSettingService( - context, - { APIService: API, onDisplayLanguageChanged: setLang }, - localStoragePath - ); + const setting = new InjectableSettingService(context, { + APIService: API, + onDisplayLanguageChanged: setLang, + deviceValues: createNodeLocalStorage(localStoragePath), + }); const appLifecycle = new NodeAppLifecycleService(context, { settingService: setting, @@ -132,10 +132,7 @@ export class NodeServiceHub extends InjectableServ setting, }); - const config = new ConfigServiceBrowserCompat(context, { - settingService: setting, - APIService: API, - }); + const config = setting.config; const replicator = new InjectableReplicatorService(context, { settingService: setting, diff --git a/src/apps/cli/services/NodeSettingService.ts b/src/apps/cli/services/NodeSettingService.ts deleted file mode 100644 index 771cb41c..00000000 --- a/src/apps/cli/services/NodeSettingService.ts +++ /dev/null @@ -1,42 +0,0 @@ -import { EVENT_SETTING_SAVED } from "@vrtmrz/livesync-commonlib/compat/events/coreEvents"; -import { EVENT_REQUEST_RELOAD_SETTING_TAB } from "@/common/events"; -import { handlers } from "@vrtmrz/livesync-commonlib/compat/services/lib/HandlerUtils"; -import type { ObsidianLiveSyncSettings } from "@vrtmrz/livesync-commonlib/compat/common/types"; -import type { ServiceContext } from "@vrtmrz/livesync-commonlib/context"; -import { SettingService, type SettingServiceDependencies } from "@vrtmrz/livesync-commonlib/compat/services/base/SettingService"; -import { - configureNodeLocalStorage, - deleteNodeLocalStorageItem, - getNodeLocalStorageItem, - setNodeLocalStorageItem, -} from "./NodeLocalStorage"; - -export class NodeSettingService extends SettingService { - constructor(context: T, dependencies: SettingServiceDependencies, storagePath: string) { - super(context, dependencies); - configureNodeLocalStorage(storagePath); - this.onSettingSaved.addHandler((settings) => { - this.context.events.emitEvent(EVENT_SETTING_SAVED, settings); - return Promise.resolve(true); - }); - this.onSettingLoaded.addHandler((settings) => { - this.context.events.emitEvent(EVENT_REQUEST_RELOAD_SETTING_TAB); - return Promise.resolve(true); - }); - } - - protected setItem(key: string, value: string) { - setNodeLocalStorageItem(key, value); - } - - protected getItem(key: string): string { - return getNodeLocalStorageItem(key); - } - - protected deleteItem(key: string): void { - deleteNodeLocalStorageItem(key); - } - - public saveData = handlers<{ saveData: (data: ObsidianLiveSyncSettings) => Promise }>().binder("saveData"); - public loadData = handlers<{ loadData: () => Promise }>().binder("loadData"); -} diff --git a/src/apps/webapp/WebAppRuntime.ts b/src/apps/webapp/WebAppRuntime.ts index 48405a4c..fbee662b 100644 --- a/src/apps/webapp/WebAppRuntime.ts +++ b/src/apps/webapp/WebAppRuntime.ts @@ -21,6 +21,8 @@ import { useCheckRemoteSize } from "@vrtmrz/livesync-commonlib/compat/serviceFea import { useRemoteConfiguration } from "@vrtmrz/livesync-commonlib/compat/serviceFeatures/remoteConfig"; import { useP2PReplicatorFeature } from "@vrtmrz/livesync-commonlib/compat/replication/trystero/useP2PReplicatorFeature"; import type { UseP2PReplicatorResult } from "@vrtmrz/livesync-commonlib/compat/replication/trystero/UseP2PReplicatorResult"; +import type { ValueStore, ValueDeleter } from "@vrtmrz/value-store"; +import { createWebStorageValueStore } from "@vrtmrz/value-store/web-storage"; import { compatGlobal } from "@vrtmrz/livesync-commonlib/compat/common/coreEnvFunctions"; import { createLiveSyncBrowserServiceHub, @@ -56,6 +58,7 @@ const DEFAULT_SETTINGS: Partial = { export type WebAppRuntimeStatusKind = "info" | "warning" | "error" | "success"; export interface WebAppRuntimeOptions { + deviceValues?: ValueStore & ValueDeleter; reportStatus?: (kind: WebAppRuntimeStatusKind, message: string) => void; scheduleReload?: (delayMilliseconds: number) => void; } @@ -71,7 +74,10 @@ export class WebAppRuntime { private paneHost: P2PReplicatorPaneHost | null = null; private restartScheduled = false; - constructor(rootHandle: FileSystemDirectoryHandle, options: WebAppRuntimeOptions = {}) { + constructor( + rootHandle: FileSystemDirectoryHandle, + private readonly options: WebAppRuntimeOptions = {} + ) { this.rootHandle = rootHandle; this.reportStatus = options.reportStatus ?? (() => {}); this.scheduleReload = @@ -89,6 +95,8 @@ export class WebAppRuntime { private createServiceOptions(): LiveSyncBrowserServiceHubOptions { return { + deviceValues: + this.options.deviceValues ?? createWebStorageValueStore(compatGlobal.localStorage, { prefix: "" }), getSystemVaultName: () => this.rootHandle.name || "livesync-webapp", settings: { save: async (data) => { @@ -219,7 +227,9 @@ export class WebAppRuntime { useCheckRemoteSize(core); useRemoteConfiguration(core); this.p2p = useP2PReplicatorFeature(core, undefined, undefined, { - prepareP2PSettings: useP2PSettingsPreparation(core.services.API.webCompatFetch.bind(core.services.API)), + prepareP2PSettings: useP2PSettingsPreparation( + core.services.API.webCompatFetch.bind(core.services.API) + ), }); this.paneHost = { services: core.services, diff --git a/src/apps/webpeer/src/WebPeerRuntime.ts b/src/apps/webpeer/src/WebPeerRuntime.ts index b4f69758..fd29947a 100644 --- a/src/apps/webpeer/src/WebPeerRuntime.ts +++ b/src/apps/webpeer/src/WebPeerRuntime.ts @@ -7,6 +7,8 @@ import { P2PLogCollector } from "@vrtmrz/livesync-commonlib/compat/replication/t import { useP2PReplicatorFeature } from "@vrtmrz/livesync-commonlib/compat/replication/trystero/useP2PReplicatorFeature"; import { ServiceContext, type LiveSyncEventHub } from "@vrtmrz/livesync-commonlib/context"; import type { P2PServiceViews } from "@vrtmrz/livesync-commonlib/p2p"; +import type { ValueStore, ValueDeleter } from "@vrtmrz/value-store"; +import { createWebStorageValueStore } from "@vrtmrz/value-store/web-storage"; import { unique } from "octagonal-wheels/collection"; import type { SimpleStore } from "octagonal-wheels/databases/SimpleStoreBase"; @@ -20,6 +22,7 @@ import { translateLiveSyncMessage } from "@/common/translation"; import { WEBPEER_STORE_NAME, createWebPeerPersistence } from "./WebPeerPersistence"; export interface WebPeerRuntimeOptions { + deviceValues?: ValueStore & ValueDeleter; context?: ServiceContext; store?: SimpleStore; deviceName?: string; @@ -62,6 +65,7 @@ export class WebPeerRuntime { this.context = options.context ?? new ServiceContext({ translate: translateLiveSyncMessage }); this.services = createLiveSyncBrowserServiceHub({ context: this.context, + deviceValues: options.deviceValues ?? createWebStorageValueStore(compatGlobal.localStorage, { prefix: "" }), getSystemVaultName: () => options.systemVaultName ?? WEBPEER_STORE_NAME, settings: persistence.settings, restart: { @@ -103,7 +107,7 @@ export class WebPeerRuntime { await this.services.setting.loadSettings(); const deviceName = this.options.deviceName?.trim(); if (deviceName) { - this.services.config.setSmallConfig(SETTING_KEY_P2P_DEVICE_NAME, deviceName); + await this.services.config.setValue(SETTING_KEY_P2P_DEVICE_NAME, deviceName); } const opened = await this.services.database.openDatabase({ replicator: this.services.replicator, @@ -192,7 +196,7 @@ export class WebPeerRuntime { ); } - getDeviceName(): string { - return this.services.config.getSmallConfig(SETTING_KEY_P2P_DEVICE_NAME) || this.services.vault.getVaultName(); + async getDeviceName(): Promise { + return (await this.services.config.getValue(SETTING_KEY_P2P_DEVICE_NAME)) || this.services.vault.getVaultName(); } } diff --git a/src/common/replicatorProviders.ts b/src/common/replicatorProviders.ts index 845d8c2e..f3cc40b0 100644 --- a/src/common/replicatorProviders.ts +++ b/src/common/replicatorProviders.ts @@ -28,6 +28,8 @@ import { getObjectStorageReplicatorConfigurationIdentity, } from "./replicatorConfigurationIdentity"; import { + createCouchDBJoinedDevicesResourceFactory, + createObjectStorageJoinedDevicesResourceFactory, createCouchDBConnectionProbeFactory, createCouchDBPreferredTweakProbeFactory, createCouchDBSecuritySeedResourceFactory, @@ -115,6 +117,9 @@ export function createCentralReplicatorProviderDefinitions( configurationIdentity: getCouchDBReplicatorConfigurationIdentity, create: () => Promise.resolve(new LiveSyncCouchDBReplicator(host)), remoteResources: { + [REMOTE_RESOURCE_KINDS.JOINED_DEVICES]: supportedCapability( + createCouchDBJoinedDevicesResourceFactory(host) + ), [REMOTE_RESOURCE_KINDS.CONNECTION]: supportedCapability(createCouchDBConnectionProbeFactory(host)), [REMOTE_RESOURCE_KINDS.PREFERRED_TWEAK]: supportedCapability( createCouchDBPreferredTweakProbeFactory(host) @@ -141,6 +146,9 @@ export function createCentralReplicatorProviderDefinitions( configurationIdentity: getObjectStorageReplicatorConfigurationIdentity, create: () => Promise.resolve(new LiveSyncJournalReplicator(host)), remoteResources: { + [REMOTE_RESOURCE_KINDS.JOINED_DEVICES]: supportedCapability( + createObjectStorageJoinedDevicesResourceFactory(host) + ), [REMOTE_RESOURCE_KINDS.CONNECTION]: supportedCapability( createObjectStorageConnectionProbeFactory(host) ), diff --git a/src/common/replicatorProviders.unit.spec.ts b/src/common/replicatorProviders.unit.spec.ts index 54a2c441..97371072 100644 --- a/src/common/replicatorProviders.unit.spec.ts +++ b/src/common/replicatorProviders.unit.spec.ts @@ -44,7 +44,16 @@ describe("central Replicator provider definitions", () => { it("keeps the retained remote-resource catalogue bounded", () => { expect .soft(Object.values(REMOTE_RESOURCE_KINDS).sort()) - .toEqual(["connection", "preferred-tweak", "security-seed", "synchronisation-information"].sort()); + .toEqual( + [ + "connection", + "initial-fetch-fast-forward", + "joined-devices", + "preferred-tweak", + "security-seed", + "synchronisation-information", + ].sort() + ); }); it("lets Journal prepare its own fresh Security Seed while CouchDB uses central preparation", () => { @@ -106,8 +115,15 @@ describe("central Replicator provider definitions", () => { const couchAdministration = definitions.get(REMOTE_COUCHDB)?.centralRemoteAdministration; const objectAdministration = definitions.get(REMOTE_MINIO)?.centralRemoteAdministration; - expect(Object.keys(couchResources ?? {}).sort()).toEqual(Object.values(REMOTE_RESOURCE_KINDS).sort()); - expect(Object.keys(objectResources ?? {}).sort()).toEqual(Object.values(REMOTE_RESOURCE_KINDS).sort()); + const requiredResources = Object.values(REMOTE_RESOURCE_KINDS).filter( + (kind) => kind !== REMOTE_RESOURCE_KINDS.INITIAL_FETCH_FAST_FORWARD + ); + expect(Object.keys(couchResources ?? {}).sort()).toEqual(requiredResources.sort()); + expect(Object.keys(objectResources ?? {}).sort()).toEqual(requiredResources.sort()); + expect(couchResources?.[REMOTE_RESOURCE_KINDS.JOINED_DEVICES]?.kind).toBe(CAPABILITY_SUPPORT_KINDS.SUPPORTED); + expect(objectResources?.[REMOTE_RESOURCE_KINDS.JOINED_DEVICES]?.kind).toBe(CAPABILITY_SUPPORT_KINDS.SUPPORTED); + expect(couchResources?.[REMOTE_RESOURCE_KINDS.INITIAL_FETCH_FAST_FORWARD]).toBeUndefined(); + expect(objectResources?.[REMOTE_RESOURCE_KINDS.INITIAL_FETCH_FAST_FORWARD]).toBeUndefined(); expect(couchResources?.[REMOTE_RESOURCE_KINDS.CONNECTION].kind).toBe(CAPABILITY_SUPPORT_KINDS.SUPPORTED); expect(couchResources?.[REMOTE_RESOURCE_KINDS.PREFERRED_TWEAK].kind).toBe(CAPABILITY_SUPPORT_KINDS.SUPPORTED); expect(couchResources?.[REMOTE_RESOURCE_KINDS.SECURITY_SEED].kind).toBe(CAPABILITY_SUPPORT_KINDS.SUPPORTED); diff --git a/src/common/replicatorResources.unit.spec.ts b/src/common/replicatorResources.unit.spec.ts index 2215a48a..0dffd7d0 100644 --- a/src/common/replicatorResources.unit.spec.ts +++ b/src/common/replicatorResources.unit.spec.ts @@ -12,6 +12,8 @@ const mocks = vi.hoisted(() => ({ getRemoteStatus: ReturnType; getRemotePreferredTweakValues: ReturnType; getReplicationPBKDF2Salt: ReturnType; + getJoinedDevices: ReturnType; + forgetJoinedDevice: ReturnType; closeReplication: ReturnType; }>, objectStorage: [] as Array<{ @@ -20,6 +22,8 @@ const mocks = vi.hoisted(() => ({ getRemoteStatus: ReturnType; getRemotePreferredTweakValues: ReturnType; getReplicationPBKDF2Salt: ReturnType; + getJoinedDevices: ReturnType; + forgetJoinedDevice: ReturnType; closeReplication: ReturnType; }>, checkSyncInfo: vi.fn(async () => true), @@ -42,6 +46,8 @@ vi.mock("@vrtmrz/livesync-commonlib/compat/replication/couchdb/LiveSyncReplicato getRemoteStatus = vi.fn(); getRemotePreferredTweakValues = vi.fn(); getReplicationPBKDF2Salt = vi.fn(); + getJoinedDevices = vi.fn(); + forgetJoinedDevice = vi.fn(); closeReplication = vi.fn(); constructor(host: unknown) { @@ -58,6 +64,8 @@ vi.mock("@vrtmrz/livesync-commonlib/compat/replication/journal/LiveSyncJournalRe getRemoteStatus = vi.fn(); getRemotePreferredTweakValues = vi.fn(); getReplicationPBKDF2Salt = vi.fn(); + getJoinedDevices = vi.fn(); + forgetJoinedDevice = vi.fn(); closeReplication = vi.fn(); constructor(host: unknown) { @@ -69,10 +77,12 @@ vi.mock("@vrtmrz/livesync-commonlib/compat/replication/journal/LiveSyncJournalRe import { createCouchDBConnectionProbeFactory, + createCouchDBJoinedDevicesResourceFactory, createCouchDBPreferredTweakProbeFactory, createCouchDBSecuritySeedResourceFactory, createCouchDBSynchronisationInformationResourceFactory, createObjectStorageConnectionProbeFactory, + createObjectStorageJoinedDevicesResourceFactory, createObjectStoragePreferredTweakProbeFactory, createObjectStorageSecuritySeedResourceFactory, } from "./replicatorResources"; @@ -226,6 +236,65 @@ describe("replicator probe factories", () => { expect(preferredReplicator.getRemotePreferredTweakValues).toHaveBeenCalledWith(snapshot); }); + it.each([ + { + kind: "CouchDB", + remoteType: REMOTE_COUCHDB, + createFactory: createCouchDBJoinedDevicesResourceFactory, + instances: () => mocks.couchDB, + }, + { + kind: "Object Storage", + remoteType: REMOTE_MINIO, + createFactory: createObjectStorageJoinedDevicesResourceFactory, + instances: () => mocks.objectStorage, + }, + ])( + "keeps $kind joined-device operations on an owned settings snapshot", + async ({ remoteType, createFactory, instances }) => { + const host = { name: "host" }; + const source = createSettings({ remoteType }); + const snapshot = { ...source }; + const factory = createFactory(host as never); + const resource = await factory(source); + const nextResource = await factory(source); + const [replicator, nextReplicator] = instances(); + expect(instances()).toHaveLength(2); + expect(replicator.host).toBe(host); + expect(replicator.getJoinedDevices).not.toHaveBeenCalled(); + expect(replicator.forgetJoinedDevice).not.toHaveBeenCalled(); + const devices = { + milestonePresent: true, + locked: false, + cleaned: false, + devices: [{ id: "other", accepted: true }], + }; + replicator.getJoinedDevices.mockResolvedValue(devices); + replicator.forgetJoinedDevice.mockResolvedValue("conflict"); + + source.couchDB_DBNAME = "changed-vault"; + source.bucket = "changed-bucket"; + await expect(resource.read()).resolves.toBe(devices); + await expect(resource.forget("other")).resolves.toBe("conflict"); + expect(replicator.getJoinedDevices).toHaveBeenCalledWith(snapshot); + expect(replicator.getJoinedDevices.mock.calls[0][0]).not.toBe(source); + expect(replicator.forgetJoinedDevice).toHaveBeenCalledWith(snapshot, "other"); + + const failure = new Error("remote unavailable"); + replicator.getJoinedDevices.mockRejectedValue(failure); + await expect(resource.read()).rejects.toBe(failure); + replicator.forgetJoinedDevice.mockRejectedValue(failure); + await expect(resource.forget("other")).rejects.toBe(failure); + const disposal = resource.dispose(); + expect(resource.dispose()).toBe(disposal); + await disposal; + expect(replicator.closeReplication).toHaveBeenCalledOnce(); + expect(nextReplicator.closeReplication).not.toHaveBeenCalled(); + await nextResource.dispose(); + expect(nextReplicator.closeReplication).toHaveBeenCalledOnce(); + } + ); + it("shares one successful asynchronous disposal promise for every probe kind", async () => { const couchProbe = await createCouchDBPreferredTweakProbeFactory({} as never)(createSettings()); const objectProbe = await createObjectStoragePreferredTweakProbeFactory({} as never)( diff --git a/src/common/replicatorResources/index.ts b/src/common/replicatorResources/index.ts index 9888e85c..f2f359f1 100644 --- a/src/common/replicatorResources/index.ts +++ b/src/common/replicatorResources/index.ts @@ -14,3 +14,8 @@ export { type SecuritySeedResourceHost, } from "./securitySeed"; export { createCouchDBSynchronisationInformationResourceFactory } from "./synchronisationInformation"; + +export { + createCouchDBJoinedDevicesResourceFactory, + createObjectStorageJoinedDevicesResourceFactory, +} from "./joinedDevices"; diff --git a/src/common/replicatorResources/joinedDevices.ts b/src/common/replicatorResources/joinedDevices.ts new file mode 100644 index 00000000..a9acf1c1 --- /dev/null +++ b/src/common/replicatorResources/joinedDevices.ts @@ -0,0 +1,40 @@ +import type { RemoteDBSettings } from "@vrtmrz/livesync-commonlib/compat/common/types"; +import type { + JoinedDevicesResource, + JoinedDevicesResourceFactory, + JoinedDevicesSnapshot, + ForgetJoinedDeviceResult, +} from "@vrtmrz/livesync-commonlib/replication"; +import { + LiveSyncCouchDBReplicator, + type LiveSyncCouchDBReplicatorEnv, +} from "@vrtmrz/livesync-commonlib/compat/replication/couchdb/LiveSyncReplicator"; +import { LiveSyncJournalReplicator } from "@vrtmrz/livesync-commonlib/compat/replication/journal/LiveSyncJournalReplicator"; +import { createReplicatorDisposer, snapshotRemoteSettings, type ResourceReplicator } from "./shared"; + +interface JoinedDevicesReplicator extends ResourceReplicator { + getJoinedDevices(setting: RemoteDBSettings): Promise; + forgetJoinedDevice(setting: RemoteDBSettings, deviceId: string): Promise; +} + +function createResource(replicator: JoinedDevicesReplicator, snapshot: RemoteDBSettings): JoinedDevicesResource { + return { + read: () => replicator.getJoinedDevices(snapshot), + forget: (deviceId) => replicator.forgetJoinedDevice(snapshot, deviceId), + dispose: createReplicatorDisposer(replicator), + }; +} + +export function createCouchDBJoinedDevicesResourceFactory( + host: LiveSyncCouchDBReplicatorEnv +): JoinedDevicesResourceFactory { + return (setting) => + Promise.resolve(createResource(new LiveSyncCouchDBReplicator(host), snapshotRemoteSettings(setting))); +} + +export function createObjectStorageJoinedDevicesResourceFactory( + host: LiveSyncCouchDBReplicatorEnv +): JoinedDevicesResourceFactory { + return (setting) => + Promise.resolve(createResource(new LiveSyncJournalReplicator(host), snapshotRemoteSettings(setting))); +} diff --git a/src/common/turnSettingsPrivacy.unit.spec.ts b/src/common/turnSettingsPrivacy.unit.spec.ts index c0243c79..451db93b 100644 --- a/src/common/turnSettingsPrivacy.unit.spec.ts +++ b/src/common/turnSettingsPrivacy.unit.spec.ts @@ -1,3 +1,4 @@ +import { MemoryValueStore } from "@vrtmrz/value-store"; import { describe, expect, it } from "vitest"; import { DEFAULT_SETTINGS, @@ -18,17 +19,7 @@ import { } from "./turnSettingsPrivacy"; class MemorySettingService extends SettingService { - readonly items = new Map(); saved?: ObsidianLiveSyncSettings; - protected setItem(key: string, value: string) { - this.items.set(key, value); - } - protected getItem(key: string) { - return this.items.get(key) ?? ""; - } - protected deleteItem(key: string) { - this.items.delete(key); - } protected saveData(settings: ObsidianLiveSyncSettings) { this.saved = structuredClone(settings); return Promise.resolve(); @@ -70,6 +61,7 @@ describe("managed TURN settings privacy", () => { const originalURI = ConnectionStringParser.serialize({ type: "p2p", settings: current }); current.remoteConfigurations.managed.uri = originalURI; const service = new MemorySettingService(new ServiceContext(), { + deviceValues: new MemoryValueStore(), APIService: { getSystemVaultName: () => "test-vault", getAppID: () => "test-app", diff --git a/src/features/P2PSync/P2PReplicator/P2PReplicatorPane.svelte b/src/features/P2PSync/P2PReplicator/P2PReplicatorPane.svelte index f8bf13ab..39ad12fc 100644 --- a/src/features/P2PSync/P2PReplicator/P2PReplicatorPane.svelte +++ b/src/features/P2PSync/P2PReplicator/P2PReplicatorPane.svelte @@ -1,5 +1,6 @@
+ {#each Object.values(storageErrors) as storageError} +

{storageError}

+ {/each}

{_msg("Peer to Peer Replicator")}

{_msg("P2P.Note.Summary")} @@ -345,6 +440,7 @@ type="text" placeholder="iphone-16" bind:value={eDeviceName} + disabled={!deviceNameReady} autocomplete="off" /> @@ -383,7 +479,7 @@ --> - diff --git a/src/features/ReviewHarness/ReviewHarnessView.ts b/src/features/ReviewHarness/ReviewHarnessView.ts index b116e9cf..2d1ae0f0 100644 --- a/src/features/ReviewHarness/ReviewHarnessView.ts +++ b/src/features/ReviewHarness/ReviewHarnessView.ts @@ -1,4 +1,5 @@ import { ItemView, Notice, Setting, type WorkspaceLeaf } from "@/deps"; +import { LOG_LEVEL_NOTICE, Logger } from "octagonal-wheels/common/logger"; import { REVIEW_HARNESS_SCENARIOS, type ReviewHarnessScenarioId, @@ -101,7 +102,7 @@ export class ReviewHarnessView extends ItemView { true ); this.addActionButton(actions, "Restart and return", "review-harness-restart", () => - this.controller.prepareCompatibilityReviewRestart() + this.controller.prepareCompatibilityReviewRestart().catch((error) => Logger(error, LOG_LEVEL_NOTICE)) ); } } diff --git a/src/features/ReviewHarness/reviewHarnessController.ts b/src/features/ReviewHarness/reviewHarnessController.ts index 2bde2f58..1cec69aa 100644 --- a/src/features/ReviewHarness/reviewHarnessController.ts +++ b/src/features/ReviewHarness/reviewHarnessController.ts @@ -23,9 +23,9 @@ export interface ReviewHarnessRuntime { runVaultRoundTrip(): Promise; runIdBenchmark(): Promise; runFileContentHashBenchmark(): Promise; - readContinuation(): string | null; - writeContinuation(value: string): void; - deleteContinuation(): void; + readContinuation(): Promise; + writeContinuation(value: string): Promise; + deleteContinuation(): Promise; restart(): void; reportError(error: unknown): void; copyText(value: string): Promise; @@ -68,6 +68,7 @@ export class ReviewHarnessController { private current: ReviewHarnessScenarioId | null = null; private resumedRequestId: string | null = null; private continuationError: string | null = null; + private continuationConsumption: Promise | undefined; constructor(private readonly runtime: ReviewHarnessRuntime) {} @@ -102,10 +103,20 @@ export class ReviewHarnessController { }; } - consumeContinuation(): void { - const serialised = this.runtime.readContinuation(); + consumeContinuation(): Promise { + if (this.continuationConsumption) return this.continuationConsumption; + const consumption = this.consumeContinuationFromStore(); + const trackedConsumption = consumption.finally(() => { + if (this.continuationConsumption === trackedConsumption) this.continuationConsumption = undefined; + }); + this.continuationConsumption = trackedConsumption; + return trackedConsumption; + } + + private async consumeContinuationFromStore(): Promise { + const serialised = await this.runtime.readContinuation(); if (!serialised) return; - this.runtime.deleteContinuation(); + await this.runtime.deleteContinuation(); const parsed = parsePendingReviewRun(serialised); if (!parsed.pendingRun) { this.continuationError = parsed.error ?? "The Review Harness continuation was invalid."; @@ -232,7 +243,7 @@ export class ReviewHarnessController { this.record("scenario-failed", id); } - prepareCompatibilityReviewRestart(): void { + async prepareCompatibilityReviewRestart(): Promise { const requestedAt = this.runtime.now().toISOString(); const pending = { formatVersion: 1, @@ -241,7 +252,7 @@ export class ReviewHarnessController { stage: "awaiting-restart", requestedAt, } as const; - this.runtime.writeContinuation(JSON.stringify(pending)); + await this.runtime.writeContinuation(JSON.stringify(pending)); this.results["compatibility-review"] = { status: "waiting-for-user", detail: "Restart requested. The one-shot continuation will be removed before the review resumes.", diff --git a/src/features/ReviewHarness/reviewHarnessController.unit.spec.ts b/src/features/ReviewHarness/reviewHarnessController.unit.spec.ts index cce28869..2b00cd23 100644 --- a/src/features/ReviewHarness/reviewHarnessController.unit.spec.ts +++ b/src/features/ReviewHarness/reviewHarnessController.unit.spec.ts @@ -90,14 +90,14 @@ function createRuntime(): ReviewHarnessRuntime & { detail: "File content hash measurements completed.", observations: ["SHA-256, 4 KiB: median=1.00 ms; range=1.00–1.00 ms"], })), - readContinuation() { + async readContinuation() { return this.continuation; }, - writeContinuation(value) { + async writeContinuation(value) { this.events.push("write"); this.continuation = value; }, - deleteContinuation() { + async deleteContinuation() { this.events.push("delete"); this.continuation = null; }, @@ -251,7 +251,7 @@ describe("ReviewHarnessController", () => { expect(controller.snapshot().running).toBe(false); }); - it("deletes a one-shot continuation before exposing the resumed guided step", () => { + it("deletes a one-shot continuation before exposing the resumed guided step", async () => { const runtime = createRuntime(); runtime.continuation = JSON.stringify({ formatVersion: 1, @@ -262,7 +262,7 @@ describe("ReviewHarnessController", () => { }); const controller = new ReviewHarnessController(runtime); - controller.consumeContinuation(); + await controller.consumeContinuation(); expect(runtime.events).toEqual(["delete"]); expect(controller.snapshot().results["compatibility-review"]).toMatchObject({ @@ -271,7 +271,7 @@ describe("ReviewHarnessController", () => { expect(controller.snapshot().resumedRequestId).toBe("compatibility-review-2026-07-18T11:59:00.000Z"); }); - it("does not copy rejected continuation values into the report", () => { + it("does not copy rejected continuation values into the report", async () => { const runtime = createRuntime(); runtime.continuation = JSON.stringify({ formatVersion: 1, @@ -282,7 +282,7 @@ describe("ReviewHarnessController", () => { }); const controller = new ReviewHarnessController(runtime); - controller.consumeContinuation(); + await controller.consumeContinuation(); expect(controller.snapshot().continuationError).toContain("private-vault"); expect(controller.snapshot().results["compatibility-review"].detail).toBe( @@ -292,7 +292,7 @@ describe("ReviewHarnessController", () => { expect(controller.createReport()).not.toContain("secret-note.md"); }); - it("does not accept an injected request ID which could enter the report transcript", () => { + it("does not accept an injected request ID which could enter the report transcript", async () => { const runtime = createRuntime(); runtime.continuation = JSON.stringify({ formatVersion: 1, @@ -303,18 +303,18 @@ describe("ReviewHarnessController", () => { }); const controller = new ReviewHarnessController(runtime); - controller.consumeContinuation(); + await controller.consumeContinuation(); expect(controller.snapshot().results["compatibility-review"].status).toBe("failed"); expect(controller.createReport()).not.toContain("private-vault"); expect(controller.createReport()).not.toContain("secret-note.md"); }); - it("persists the fixed continuation before requesting restart", () => { + it("persists the fixed continuation before requesting restart", async () => { const runtime = createRuntime(); const controller = new ReviewHarnessController(runtime); - controller.prepareCompatibilityReviewRestart(); + await controller.prepareCompatibilityReviewRestart(); expect(runtime.events).toEqual(["write", "restart"]); expect(JSON.parse(runtime.continuation!)).toMatchObject({ @@ -324,6 +324,135 @@ describe("ReviewHarnessController", () => { }); }); + it("waits for the continuation save before requesting restart", async () => { + const runtime = createRuntime(); + let markSaveStarted!: () => void; + const saveStarted = new Promise((resolve) => { + markSaveStarted = resolve; + }); + let finishSave!: () => void; + const saving = new Promise((resolve) => { + finishSave = resolve; + }); + runtime.writeContinuation = vi.fn(async (value) => { + runtime.events.push("write-started"); + markSaveStarted(); + await saving; + runtime.continuation = value; + runtime.events.push("write-saved"); + }); + const controller = new ReviewHarnessController(runtime); + + const preparing = controller.prepareCompatibilityReviewRestart(); + await saveStarted; + + expect(runtime.events).toEqual(["write-started"]); + expect(runtime.restart).not.toHaveBeenCalled(); + expect(controller.snapshot().results["compatibility-review"].status).toBe("idle"); + + finishSave(); + await preparing; + + expect(runtime.events).toEqual(["write-started", "write-saved", "restart"]); + expect(controller.snapshot().results["compatibility-review"].status).toBe("waiting-for-user"); + }); + + it("keeps the guided review retryable when saving its continuation fails", async () => { + const runtime = createRuntime(); + const controller = new ReviewHarnessController(runtime); + await controller.runScenario("compatibility-review"); + runtime.writeContinuation = vi.fn(async () => { + runtime.events.push("write-failed"); + throw new Error("continuation save failed"); + }); + + await expect(controller.prepareCompatibilityReviewRestart()).rejects.toThrow("continuation save failed"); + + expect(runtime.events).toEqual(["write-failed"]); + expect(runtime.restart).not.toHaveBeenCalled(); + expect(controller.snapshot().results["compatibility-review"].status).toBe("waiting-for-user"); + }); + + it("waits for one continuation deletion before exposing it and coalesces concurrent consumption", async () => { + const runtime = createRuntime(); + runtime.continuation = JSON.stringify({ + formatVersion: 1, + requestId: "compatibility-review-2026-07-18T11:59:00.000Z", + scenarioId: "compatibility-review", + stage: "awaiting-restart", + requestedAt: "2026-07-18T11:59:00.000Z", + }); + let markDeleteStarted!: () => void; + const deleteStarted = new Promise((resolve) => { + markDeleteStarted = resolve; + }); + let finishDelete!: () => void; + const deleting = new Promise((resolve) => { + finishDelete = resolve; + }); + runtime.readContinuation = vi.fn(async () => { + runtime.events.push("read"); + return runtime.continuation; + }); + runtime.deleteContinuation = vi.fn(async () => { + runtime.events.push("delete-started"); + markDeleteStarted(); + await deleting; + runtime.continuation = null; + runtime.events.push("delete-finished"); + }); + const controller = new ReviewHarnessController(runtime); + + const firstConsumption = controller.consumeContinuation(); + const duplicateConsumption = controller.consumeContinuation(); + await deleteStarted; + + expect(duplicateConsumption).toBe(firstConsumption); + expect(runtime.events).toEqual(["read", "delete-started"]); + expect(controller.snapshot().resumedRequestId).toBeNull(); + expect(controller.snapshot().results["compatibility-review"].status).toBe("idle"); + + finishDelete(); + await firstConsumption; + + expect(runtime.events).toEqual(["read", "delete-started", "delete-finished"]); + expect(runtime.readContinuation).toHaveBeenCalledOnce(); + expect(runtime.deleteContinuation).toHaveBeenCalledOnce(); + expect(controller.snapshot().resumedRequestId).toBe("compatibility-review-2026-07-18T11:59:00.000Z"); + expect(controller.snapshot().results["compatibility-review"].status).toBe("waiting-for-user"); + }); + + it("keeps a failed continuation deletion available for retry", async () => { + const runtime = createRuntime(); + runtime.continuation = JSON.stringify({ + formatVersion: 1, + requestId: "compatibility-review-2026-07-18T11:59:00.000Z", + scenarioId: "compatibility-review", + stage: "awaiting-restart", + requestedAt: "2026-07-18T11:59:00.000Z", + }); + runtime.deleteContinuation = vi + .fn() + .mockRejectedValueOnce(new Error("continuation delete failed")) + .mockImplementation(async () => { + runtime.events.push("delete"); + runtime.continuation = null; + }); + const controller = new ReviewHarnessController(runtime); + + await expect(controller.consumeContinuation()).rejects.toThrow("continuation delete failed"); + + expect(runtime.continuation).not.toBeNull(); + expect(controller.snapshot().resumedRequestId).toBeNull(); + expect(controller.snapshot().results["compatibility-review"].status).toBe("idle"); + + await controller.consumeContinuation(); + + expect(runtime.continuation).toBeNull(); + expect(controller.snapshot().resumedRequestId).toBe("compatibility-review-2026-07-18T11:59:00.000Z"); + expect(controller.snapshot().results["compatibility-review"].status).toBe("waiting-for-user"); + }); + it("passes only after the actual compatibility review clears its pause", async () => { const runtime = createRuntime(); const controller = new ReviewHarnessController(runtime); diff --git a/src/modules/features/ModuleObsidianSettingTab.ts b/src/modules/features/ModuleObsidianSettingTab.ts index 727eee81..ccb51bbd 100644 --- a/src/modules/features/ModuleObsidianSettingTab.ts +++ b/src/modules/features/ModuleObsidianSettingTab.ts @@ -8,13 +8,13 @@ import { openObsidianSettings } from "@/common/obsidianSettings.ts"; export class ModuleObsidianSettingDialogue extends AbstractObsidianModule { settingTab!: ObsidianLiveSyncSettingTab; - _everyOnloadAfterLoadSettings(): Promise { + async _everyOnloadAfterLoadSettings(): Promise { this.settingTab = new ObsidianLiveSyncSettingTab(this.app, this.plugin); - this.settingTab.reloadAllSettings(true); + await this.settingTab.loadSettings(); this.plugin.addSettingTab(this.settingTab); eventHub.onEvent(EVENT_REQUEST_OPEN_SETTINGS, () => this.openSetting()); - return Promise.resolve(true); + return true; } openSetting() { diff --git a/src/modules/features/ModuleObsidianSettingTab.unit.spec.ts b/src/modules/features/ModuleObsidianSettingTab.unit.spec.ts index b1840ea0..87ddc9eb 100644 --- a/src/modules/features/ModuleObsidianSettingTab.unit.spec.ts +++ b/src/modules/features/ModuleObsidianSettingTab.unit.spec.ts @@ -2,7 +2,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; const settingTabState = vi.hoisted(() => ({ callOrder: [] as string[], - reloadAllSettings: vi.fn<(skipUpdate?: boolean) => void>(), + loadSettings: vi.fn<() => Promise>(), })); const eventHubState = vi.hoisted(() => ({ @@ -11,9 +11,9 @@ const eventHubState = vi.hoisted(() => ({ vi.mock("./SettingDialogue/ObsidianLiveSyncSettingTab.ts", () => ({ ObsidianLiveSyncSettingTab: class ObsidianLiveSyncSettingTab { - reloadAllSettings(skipUpdate?: boolean) { - settingTabState.callOrder.push(`reload:${String(skipUpdate)}`); - settingTabState.reloadAllSettings(skipUpdate); + async loadSettings() { + await settingTabState.loadSettings(); + settingTabState.callOrder.push("settings-loaded"); } }, })); @@ -65,7 +65,7 @@ function createModuleHarness() { describe("ModuleObsidianSettingDialogue startup lifecycle", () => { beforeEach(() => { settingTabState.callOrder.length = 0; - settingTabState.reloadAllSettings.mockClear(); + settingTabState.loadSettings.mockReset().mockResolvedValue(undefined); eventHubState.onEvent.mockClear(); }); @@ -79,13 +79,29 @@ describe("ModuleObsidianSettingDialogue startup lifecycle", () => { }); it("seeds the setting editor without requesting a render before registration", async () => { - const { initialisationHandler, settingsLoadedHandler } = createModuleHarness(); + const { initialisationHandler, settingsLoadedHandler, plugin } = createModuleHarness(); + let finishRead!: () => void; + settingTabState.loadSettings.mockReturnValueOnce( + new Promise((resolve) => { + finishRead = resolve; + }) + ); const handler = settingsLoadedHandler() ?? initialisationHandler(); expect(handler).toBeTypeOf("function"); - await handler!(); + const loading = handler!(); + expect(plugin.addSettingTab).not.toHaveBeenCalled(); + finishRead(); + await loading; - expect(settingTabState.reloadAllSettings).toHaveBeenCalledWith(true); - expect(settingTabState.callOrder).toEqual(["reload:true", "add-setting-tab"]); + expect(settingTabState.loadSettings).toHaveBeenCalledOnce(); + expect(settingTabState.callOrder).toEqual(["settings-loaded", "add-setting-tab"]); + }); + it("does not register an editor when reading device values fails", async () => { + const { settingsLoadedHandler, plugin } = createModuleHarness(); + const failure = new Error("device storage unavailable"); + settingTabState.loadSettings.mockRejectedValueOnce(failure); + await expect(settingsLoadedHandler()!()).rejects.toBe(failure); + expect(plugin.addSettingTab).not.toHaveBeenCalled(); }); }); diff --git a/src/modules/features/SettingDialogue/ObsidianLiveSyncSettingTab.declarative.unit.spec.ts b/src/modules/features/SettingDialogue/ObsidianLiveSyncSettingTab.declarative.unit.spec.ts index d45582bb..da894f79 100644 --- a/src/modules/features/SettingDialogue/ObsidianLiveSyncSettingTab.declarative.unit.spec.ts +++ b/src/modules/features/SettingDialogue/ObsidianLiveSyncSettingTab.declarative.unit.spec.ts @@ -1,3 +1,4 @@ +import { MemoryValueStore } from "@vrtmrz/value-store"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { DEFAULT_SETTINGS } from "@vrtmrz/livesync-commonlib/compat/common/types"; import type { SettingDefinitionGroup, SettingDefinitionItem, SettingDefinitionPage } from "obsidian"; @@ -158,6 +159,7 @@ function createSettingsTab(options: SettingsTabOptions = {}): ObsidianLiveSyncSe }, services: { setting: { + deviceValues: new MemoryValueStore(), getDeviceAndVaultName: vi.fn(() => ""), saveSettingData: vi.fn(async () => undefined), }, @@ -457,7 +459,7 @@ describe("ObsidianLiveSyncSettingTab native page lifecycle", () => { expect(runtime.pageCleanup).toHaveBeenCalledOnce(); }); - it("rebuilds the catalogue when an externally loaded setting changes page visibility", () => { + it("rebuilds the catalogue when an externally loaded setting changes page visibility", async () => { const tab = createSettingsTab(); const changeLog = findPage(tab, "Change Log"); if (!changeLog.page) { @@ -466,7 +468,7 @@ describe("ObsidianLiveSyncSettingTab native page lifecycle", () => { changeLog.page().display(); tab.core.settings.usePowerUserMode = !tab.editingSettings.usePowerUserMode; - tab.requestReload(); + await tab.requestReload(); expect(tab.update).toHaveBeenCalledOnce(); }); @@ -480,7 +482,7 @@ describe("ObsidianLiveSyncSettingTab native page lifecycle", () => { expect(tab.update).toHaveBeenCalledOnce(); }); - it("rebuilds translated catalogue names when the display language changes externally", () => { + it("rebuilds translated catalogue names when the display language changes externally", async () => { const tab = createSettingsTab(); const changeLog = findPage(tab, "Change Log"); if (!changeLog.page) { @@ -489,12 +491,12 @@ describe("ObsidianLiveSyncSettingTab native page lifecycle", () => { changeLog.page().display(); tab.core.settings.displayLanguage = "ja"; - tab.requestReload(); + await tab.requestReload(); expect(tab.update).toHaveBeenCalledOnce(); }); - it("rebuilds the catalogue after accepting an external page-visibility setting over a dirty value", () => { + it("rebuilds the catalogue after accepting an external page-visibility setting over a dirty value", async () => { const tab = createSettingsTab(); const changeLog = findPage(tab, "Change Log"); if (!changeLog.page) { @@ -505,7 +507,7 @@ describe("ObsidianLiveSyncSettingTab native page lifecycle", () => { tab.editingSettings.usePowerUserMode = true; tab.core.settings.usePowerUserMode = true; - tab.requestReload(); + await tab.requestReload(); const configureAnchor = vi.mocked(tab.core.confirm.askInPopup).mock.calls[0]?.[2]; expect(configureAnchor).toBeTypeOf("function"); let acceptExternalSetting: (() => void) | undefined; diff --git a/src/modules/features/SettingDialogue/ObsidianLiveSyncSettingTab.ts b/src/modules/features/SettingDialogue/ObsidianLiveSyncSettingTab.ts index 424213f8..a3c72a31 100644 --- a/src/modules/features/SettingDialogue/ObsidianLiveSyncSettingTab.ts +++ b/src/modules/features/SettingDialogue/ObsidianLiveSyncSettingTab.ts @@ -52,7 +52,6 @@ import { type PageFunctions, type UpdateFunction, } from "./SettingPane.ts"; -import { compatGlobal } from "@vrtmrz/livesync-commonlib/compat/common/coreEnvFunctions"; import { JournalSyncCore } from "@vrtmrz/livesync-commonlib/compat/replication/journal/JournalSyncCore"; import { MinioStorageAdapter } from "@vrtmrz/livesync-commonlib/compat/replication/journal/objectstore/MinioStorageAdapter"; import { closeObsidianSettings } from "@/common/obsidianSettings.ts"; @@ -100,6 +99,9 @@ export class ObsidianLiveSyncSettingTab extends PluginSettingTab { } selectedScreen = ""; + private localConfigurationPassphrase = ""; + private localSettingsReadVersion = 0; + _editingSettings?: AllSettings; // Buffered Settings for editing get editingSettings(): AllSettings { @@ -162,8 +164,7 @@ export class ObsidianLiveSyncSettingTab extends PluginSettingTab { async saveLocalSetting(key: keyof typeof OnDialogSettingsDefault) { if (key == "deviceAndVaultName") { this.services.setting.setDeviceAndVaultName(this.editingSettings?.[key] ?? ""); - this.services.setting.saveDeviceAndVaultName(); - return await Promise.resolve(); + await this.services.setting.saveDeviceAndVaultName(); } } /** @@ -225,6 +226,10 @@ export class ObsidianLiveSyncSettingTab extends PluginSettingTab { appliedConfigurationEncryption.configPassphrase ); + if (appliedConfigurationEncryption.configPassphraseStore === "LOCALSTORAGE") { + this.localSettingsReadVersion++; + this.localConfigurationPassphrase = appliedConfigurationEncryption.configPassphrase; + } this.copySettingValue(this.initialSettings, appliedConfigurationEncryption, "configPassphraseStore"); this.copySettingValue(this.initialSettings, appliedConfigurationEncryption, "configPassphrase"); } @@ -285,9 +290,21 @@ export class ObsidianLiveSyncSettingTab extends PluginSettingTab { } } + private async readLocalSettings(): Promise { + const version = ++this.localSettingsReadVersion; + const passphrase = await this.services.setting.deviceValues.getValue("ls-setting-passphrase", ""); + if (version !== this.localSettingsReadVersion) return false; + this.localConfigurationPassphrase = passphrase; + return true; + } + + async loadSettings(): Promise { + if (await this.readLocalSettings()) this.reloadAllSettings(true); + } + reloadAllLocalSettings() { const ret = { ...OnDialogSettingsDefault }; - ret.configPassphrase = compatGlobal.localStorage.getItem("ls-setting-passphrase") || ""; + ret.configPassphrase = this.localConfigurationPassphrase; ret.preset = ""; ret.deviceAndVaultName = this.services.setting.getDeviceAndVaultName(); return ret; @@ -402,7 +419,7 @@ export class ObsidianLiveSyncSettingTab extends PluginSettingTab { this.plugin = plugin; Setting.env = this; eventHub.onEvent(EVENT_REQUEST_RELOAD_SETTING_TAB, () => { - this.requestReload(); + void this.requestReload().catch((error: unknown) => Logger(error, LOG_LEVEL_NOTICE)); }); this.addOnSaved("displayLanguage", () => this.requestCatalogueRefresh()); this.addOnSaved("showStatusOnEditor", () => eventHub.emitEvent(EVENT_ON_UNRESOLVED_ERROR)); @@ -545,7 +562,8 @@ export class ObsidianLiveSyncSettingTab extends PluginSettingTab { ); } - requestReload() { + async requestReload(): Promise { + if (!(await this.readLocalSettings())) return; const nativeTabIsShown = this.supportsDeclarativeSettings() && this.containerEl !== undefined && this.containerEl.isShown(); if (this.isShown || nativeTabIsShown) { diff --git a/src/modules/features/SettingDialogue/ObsidianLiveSyncSettingTab.unit.spec.ts b/src/modules/features/SettingDialogue/ObsidianLiveSyncSettingTab.unit.spec.ts index 1349edfc..df8f6b23 100644 --- a/src/modules/features/SettingDialogue/ObsidianLiveSyncSettingTab.unit.spec.ts +++ b/src/modules/features/SettingDialogue/ObsidianLiveSyncSettingTab.unit.spec.ts @@ -1,7 +1,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { DEFAULT_SETTINGS, LOG_LEVEL_NOTICE, REMOTE_COUCHDB } from "@vrtmrz/livesync-commonlib/compat/common/types"; import { REMOTE_RESOURCE_KINDS } from "@vrtmrz/livesync-commonlib/replication"; -import { compatGlobal } from "@vrtmrz/livesync-commonlib/compat/common/coreEnvFunctions"; +import { MemoryValueStore } from "@vrtmrz/value-store"; const settingsInitialisationMocks = vi.hoisted(() => ({ applySettingsWithInitialisationChoice: vi.fn(), @@ -76,8 +76,6 @@ beforeEach(() => { settingsInitialisationMocks.applySettingsWithInitialisationChoice.mockReset(); loggerMocks.Logger.mockClear(); vi.mocked($msg).mockClear(); - vi.mocked(compatGlobal.localStorage.getItem).mockReset().mockReturnValue(null); - vi.mocked(compatGlobal.localStorage.setItem).mockReset(); }); describe("ObsidianLiveSyncSettingTab passphrase verification", () => { @@ -280,6 +278,7 @@ describe("ObsidianLiveSyncSettingTab pending-setting initialisation", () => { }, services: { setting: { + deviceValues: new MemoryValueStore(), saveSettingData, getDeviceAndVaultName: vi.fn(() => ""), suspendAllSync, @@ -665,6 +664,7 @@ describe("ObsidianLiveSyncSettingTab declarative settings boundary", () => { }, services: { setting: { + deviceValues: new MemoryValueStore(), saveSettingData, getDeviceAndVaultName: vi.fn(() => ""), }, @@ -692,6 +692,54 @@ describe("ObsidianLiveSyncSettingTab declarative settings boundary", () => { return { tab, saveSettingData }; } + it("awaits the stored passphrase before initialising the editing buffer", async () => { + const { tab } = createSettingsTab(); + tab.resetEditingSettings(); + let resolveRead!: (value: string) => void; + vi.spyOn(tab.services.setting.deviceValues, "getValue").mockReturnValueOnce( + new Promise((resolve) => { + resolveRead = resolve; + }) + ); + const loading = tab.loadSettings(); + expect(tab.initialSettings).toBeUndefined(); + resolveRead("stored passphrase"); + await loading; + expect(tab.editingSettings.configPassphrase).toBe("stored passphrase"); + expect(tab.initialSettings!.configPassphrase).toBe("stored passphrase"); + }); + + it("preserves the loaded settings when device storage cannot be read", async () => { + const { tab } = createSettingsTab(); + await tab.services.setting.deviceValues.setValue("ls-setting-passphrase", "stored passphrase"); + await tab.loadSettings(); + const failure = new Error("device storage unavailable"); + vi.spyOn(tab.services.setting.deviceValues, "getValue").mockRejectedValueOnce(failure); + await expect(tab.requestReload()).rejects.toBe(failure); + expect(tab.editingSettings.configPassphrase).toBe("stored passphrase"); + expect(tab.initialSettings!.configPassphrase).toBe("stored passphrase"); + }); + + it("does not let a delayed read overwrite a successfully saved passphrase", async () => { + const { tab } = createSettingsTab(); + await tab.loadSettings(); + let resolveRead!: (value: string) => void; + vi.spyOn(tab.services.setting.deviceValues, "getValue").mockReturnValueOnce( + new Promise((resolve) => { + resolveRead = resolve; + }) + ); + const loading = tab.loadSettings(); + Object.assign(tab.services.setting, { changeConfigurationEncryption: vi.fn(async () => undefined) }); + tab.editingSettings.configPassphraseStore = "LOCALSTORAGE"; + tab.editingSettings.configPassphrase = "new passphrase"; + await tab.saveSettings(["configPassphraseStore", "configPassphrase"]); + resolveRead("old passphrase"); + await loading; + tab.reloadAllSettings(); + expect(tab.initialSettings!.configPassphrase).toBe("new passphrase"); + }); + it("loads the imperative fallback without a SettingPage runtime export", () => { const { tab } = createSettingsTab(); @@ -779,7 +827,7 @@ describe("ObsidianLiveSyncSettingTab declarative settings boundary", () => { it("saves a custom configuration passphrase edit through the setting service", async () => { const { tab } = createSettingsTab(); const changeConfigurationEncryption = vi.fn(async () => { - vi.mocked(compatGlobal.localStorage.getItem).mockReturnValue("new custom passphrase"); + await tab.services.setting.deviceValues.setValue("ls-setting-passphrase", "new custom passphrase"); }); Object.assign(tab.core.services.setting, { changeConfigurationEncryption }); const store = "LOCALSTORAGE"; @@ -787,13 +835,13 @@ describe("ObsidianLiveSyncSettingTab declarative settings boundary", () => { tab.initialSettings!.configPassphraseStore = store; tab.editingSettings.configPassphrase = "new custom passphrase"; tab.initialSettings!.configPassphrase = "old custom passphrase"; - vi.mocked(compatGlobal.localStorage.setItem).mockClear(); + const setValue = vi.spyOn(tab.services.setting.deviceValues, "setValue"); await tab.saveSettings(["configPassphrase"]); expect(changeConfigurationEncryption).toHaveBeenCalledOnce(); expect(changeConfigurationEncryption).toHaveBeenCalledWith(store, "new custom passphrase"); - expect(compatGlobal.localStorage.setItem).not.toHaveBeenCalled(); + expect(setValue).toHaveBeenCalledOnce(); expect(tab.initialSettings!.configPassphrase).toBe("new custom passphrase"); }); @@ -811,7 +859,6 @@ describe("ObsidianLiveSyncSettingTab declarative settings boundary", () => { tab.initialSettings!.configPassphraseStore = "LOCALSTORAGE"; tab.initialSettings!.configPassphrase = "previous passphrase"; tab.editingSettings.configPassphrase = "passphrase A"; - vi.mocked(compatGlobal.localStorage.getItem).mockReturnValue("passphrase A"); const apply = tab.saveSettings(["configPassphrase"]); expect(changeConfigurationEncryption).toHaveBeenCalledWith("LOCALSTORAGE", "passphrase A"); @@ -840,13 +887,13 @@ describe("ObsidianLiveSyncSettingTab declarative settings boundary", () => { tab.editingSettings.configPassphraseStore = editedStore; tab.initialSettings!.configPassphrase = "old custom passphrase"; tab.editingSettings.configPassphrase = "new custom passphrase"; - vi.mocked(compatGlobal.localStorage.setItem).mockClear(); + const setValue = vi.spyOn(tab.services.setting.deviceValues, "setValue"); await expect(tab.saveSettings(["configPassphraseStore", "configPassphrase"])).rejects.toBe(failure); expect(changeConfigurationEncryption).toHaveBeenCalledOnce(); expect(changeConfigurationEncryption).toHaveBeenCalledWith(editedStore, "new custom passphrase"); - expect(compatGlobal.localStorage.setItem).not.toHaveBeenCalled(); + expect(setValue).not.toHaveBeenCalled(); expect(tab.core.settings.configPassphraseStore).toBe(savedStore); expect(tab.initialSettings!.configPassphraseStore).toBe(savedStore); expect(tab.initialSettings!.configPassphrase).toBe("old custom passphrase"); diff --git a/src/modules/features/SetupManager.unit.spec.ts b/src/modules/features/SetupManager.unit.spec.ts index c8dfe4ec..68320e2c 100644 --- a/src/modules/features/SetupManager.unit.spec.ts +++ b/src/modules/features/SetupManager.unit.spec.ts @@ -1,3 +1,4 @@ +import { MemoryValueStore } from "@vrtmrz/value-store"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { DEFAULT_SETTINGS, @@ -37,11 +38,6 @@ import { decodeSettingsFromQRCodeData } from "@vrtmrz/livesync-commonlib/compat/ import { SetupManager, UserMode } from "./SetupManager"; class TestSettingService extends SettingService { - protected setItem(_key: string, _value: string): void {} - protected getItem(_key: string): string { - return ""; - } - protected deleteItem(_key: string): void {} protected saveData(_setting: ObsidianLiveSyncSettings): Promise { return Promise.resolve(); } @@ -65,6 +61,7 @@ function createLegacyRemoteSetting(): ObsidianLiveSyncSettings { function createSetupManager() { const setting = new TestSettingService(new ServiceContext(), { + deviceValues: new MemoryValueStore(), APIService: { getSystemVaultName: vi.fn(() => "vault"), getAppID: vi.fn(() => "app"), diff --git a/src/modules/features/SetupWizard/dialogs/SetupRemoteP2P.svelte b/src/modules/features/SetupWizard/dialogs/SetupRemoteP2P.svelte index 2f89b845..210633b5 100644 --- a/src/modules/features/SetupWizard/dialogs/SetupRemoteP2P.svelte +++ b/src/modules/features/SetupWizard/dialogs/SetupRemoteP2P.svelte @@ -52,26 +52,48 @@ const context = getDialogContext(); let error = $state(""); let connectionPathResetNotice = $state(false); + let peerNameReady = $state(false); + let mounted = false; const hasValidTurnServer = $derived(hasP2PTurnConfiguration(syncSetting)); type Props = GuestDialogProps; const { setResult, getInitialData }: Props = $props(); let connectionProbe: SetupRemoteP2PInitialData["connectionProbe"] | undefined; onMount(() => { + mounted = true; const initialData = getInitialData?.(); connectionProbe = initialData?.connectionProbe; const initialSettings = initialData?.settings; if (initialSettings) { syncSetting = pickP2PSyncSettings(initialSettings); } - const initialPeerName = (initialSettings?.P2P_DevicePeerName ?? "").trim(); + const savedPeerName = initialSettings?.P2P_DevicePeerName; + const initialPeerName = typeof savedPeerName === "string" ? savedPeerName.trim() : ""; if (initialPeerName !== "") { - return; - } - const cachedPeerName = context.services.config.getSmallConfig(SETTING_KEY_P2P_DEVICE_NAME); - if (cachedPeerName) { - syncSetting.P2P_DevicePeerName = cachedPeerName as string; + peerNameReady = true; + return () => { + mounted = false; + }; } + void context.services.config.getValue(SETTING_KEY_P2P_DEVICE_NAME).then( + (cachedPeerName) => { + if (mounted) { + if (typeof cachedPeerName === "string" && cachedPeerName) { + syncSetting.P2P_DevicePeerName = cachedPeerName; + } + peerNameReady = true; + } + }, + (loadError: unknown) => { + if (mounted) { + error = loadError instanceof Error ? loadError.message : String(loadError); + peerNameReady = true; + } + } + ); + return () => { + mounted = false; + }; }); function generateSetting() { const connSetting: P2PSyncSetting = { @@ -193,6 +215,7 @@ } async function checkAndCommit() { + if (!peerNameReady) return; error = ""; try { error = (await checkConnection()) || ""; @@ -207,6 +230,7 @@ } } function commit() { + if (!peerNameReady) return; error = validateManagedTurnSettings(syncSetting) ?? ""; if (error) return; const setting = pickP2PSyncSettings(generateSetting()); @@ -217,6 +241,7 @@ } const canProceed = $derived.by(() => { return ( + peerNameReady && syncSetting.P2P_relays.trim() !== "" && syncSetting.P2P_roomID.trim() !== "" && syncSetting.P2P_passphrase.trim() !== "" && @@ -291,6 +316,7 @@ autocorrect="off" autocapitalize="off" spellcheck="false" + disabled={!peerNameReady} bind:value={syncSetting.P2P_DevicePeerName} /> @@ -385,7 +411,11 @@ disabled={!canProceed} commit={() => checkAndCommit()} /> - commit()} /> + commit()} + /> cancel()} /> {/if} diff --git a/src/modules/services/ObsidianServiceHub.ts b/src/modules/services/ObsidianServiceHub.ts index 3613ea27..b361986f 100644 --- a/src/modules/services/ObsidianServiceHub.ts +++ b/src/modules/services/ObsidianServiceHub.ts @@ -1,4 +1,6 @@ import { InjectableServiceHub } from "@vrtmrz/livesync-commonlib/compat/services/implements/injectable/InjectableServiceHub"; +import { compatGlobal } from "@vrtmrz/livesync-commonlib/compat/common/coreEnvFunctions"; +import { createWebStorageValueStore } from "@vrtmrz/value-store/web-storage"; import { ObsidianServiceContext } from "@/modules/services/ObsidianServiceContext"; import type { ServiceInstances } from "@vrtmrz/livesync-commonlib/compat/services/ServiceHub"; import type ObsidianLiveSyncPlugin from "@/main"; @@ -11,7 +13,6 @@ import { ObsidianTweakValueService, ObsidianTestService, ObsidianDatabaseEventService, - ObsidianConfigService, ObsidianKeyValueDBService, ObsidianControlService, } from "./ObsidianServices"; @@ -46,6 +47,7 @@ export class ObsidianServiceHub extends InjectableServiceHub( - task: () => T | PromiseLike, - options?: ActivityOptions - ): Promise { + async runBoundedLocalApplicationActivity(task: () => T | PromiseLike, options?: ActivityOptions): Promise { this.boundedLocalApplicationActivityCount.value++; try { return this.dependencies.activityRunner @@ -84,7 +80,6 @@ export class ObsidianConflictService extends InjectableConflictService {} // InjectableTestService export class ObsidianTestService extends InjectableTestService {} -export class ObsidianConfigService extends ConfigServiceBrowserCompat {} export class ObsidianKeyValueDBService extends KeyValueDBService {} diff --git a/src/modules/services/ObsidianSettingService.ts b/src/modules/services/ObsidianSettingService.ts index a934ebd1..87b93b59 100644 --- a/src/modules/services/ObsidianSettingService.ts +++ b/src/modules/services/ObsidianSettingService.ts @@ -1,4 +1,3 @@ -import { compatGlobal } from "@vrtmrz/livesync-commonlib/compat/common/coreEnvFunctions"; import { type ObsidianLiveSyncSettings } from "@vrtmrz/livesync-commonlib/compat/common/types"; import { EVENT_REQUEST_RELOAD_SETTING_TAB, @@ -27,19 +26,6 @@ export class ObsidianSettingService extends Se return Promise.resolve(true); }); } - protected setItem(key: string, value: string) { - // TODO: Implement nativeLocalStorage. - return compatGlobal.localStorage.setItem(key, value); - } - protected getItem(key: string): string { - // TODO: Implement nativeLocalStorage. - return compatGlobal.localStorage.getItem(key) ?? ""; - } - protected deleteItem(key: string): void { - // TODO: Implement nativeLocalStorage. - compatGlobal.localStorage.removeItem(key); - } - protected override async saveData(data: ObsidianLiveSyncSettings): Promise { return await this.context.liveSyncPlugin.saveData(data); } diff --git a/src/modules/services/ObsidianSettingService.unit.spec.ts b/src/modules/services/ObsidianSettingService.unit.spec.ts index 9175c931..6df8404d 100644 --- a/src/modules/services/ObsidianSettingService.unit.spec.ts +++ b/src/modules/services/ObsidianSettingService.unit.spec.ts @@ -1,6 +1,23 @@ import { describe, expect, it } from "vitest"; +import { createServiceContext } from "@vrtmrz/livesync-commonlib/context"; import type { ObsidianLiveSyncSettings } from "@vrtmrz/livesync-commonlib/compat/common/types"; +import type { IAPIService } from "@vrtmrz/livesync-commonlib/compat/services/base/IService"; +import { MemoryValueStore } from "@vrtmrz/value-store"; +import type { ObsidianServiceContext } from "@/modules/services/ObsidianServiceContext"; import { normaliseObsidianSettingsData } from "./ObsidianSettingService.ts"; +import { ObsidianSettingService } from "./ObsidianSettingService.ts"; + +function createSettingService( + vaultName: string, + deviceValues: MemoryValueStore +): ObsidianSettingService { + const context = createServiceContext() as unknown as ObsidianServiceContext; + const APIService = { + addLog: () => undefined, + getSystemVaultName: () => vaultName, + } as unknown as IAPIService; + return new ObsidianSettingService(context, { APIService, deviceValues }); +} describe("normaliseObsidianSettingsData", () => { it("maps Obsidian's missing data value to Commonlib's new-Vault input", () => { @@ -12,4 +29,26 @@ describe("normaliseObsidianSettingsData", () => { expect(normaliseObsidianSettingsData(settings)).toBe(settings); }); + + it("keeps injected device values independent from each Vault-scoped configuration store", async () => { + const firstValues = new MemoryValueStore(); + const secondValues = new MemoryValueStore(); + const firstSetting = createSettingService("first-vault", firstValues); + const secondSetting = createSettingService("second-vault", secondValues); + + await firstSetting.deviceValues.setValue("shared-key", "device value"); + await firstSetting.config.setValue("shared-key", "first config"); + await secondSetting.config.setValue("shared-key", "second config"); + + expect(firstSetting.deviceValues).toBe(firstValues); + expect(secondSetting.deviceValues).toBe(secondValues); + expect(firstSetting.deviceValues).not.toBe(secondSetting.deviceValues); + expect(firstSetting.config).not.toBe(firstValues); + expect(firstSetting.config).not.toBe(secondSetting.config); + await expect(firstSetting.deviceValues.getValue("shared-key")).resolves.toBe("device value"); + await expect(firstSetting.config.getValue("shared-key")).resolves.toBe("first config"); + await expect(secondSetting.config.getValue("shared-key")).resolves.toBe("second config"); + await expect(firstValues.getValue("first-vault-shared-key")).resolves.toBe("first config"); + await expect(secondValues.getValue("second-vault-shared-key")).resolves.toBe("second config"); + }); }); diff --git a/src/modules/services/ObsidianUIService.ts b/src/modules/services/ObsidianUIService.ts index c5abf23f..08e6e572 100644 --- a/src/modules/services/ObsidianUIService.ts +++ b/src/modules/services/ObsidianUIService.ts @@ -1,4 +1,4 @@ -import type { ConfigService } from "@vrtmrz/livesync-commonlib/compat/services/base/ConfigService"; +import type { IConfigService } from "@vrtmrz/livesync-commonlib/compat/services/base/IService"; import type { AppLifecycleService } from "@vrtmrz/livesync-commonlib/compat/services/base/AppLifecycleService"; import type { ReplicatorService } from "@vrtmrz/livesync-commonlib/compat/services/base/ReplicatorService"; import { UIService } from "@vrtmrz/livesync-commonlib/compat/services/implements/base/UIService"; @@ -8,7 +8,7 @@ import DialogToCopy from "@/modules/services/LiveSyncUI/dialogues/DialogueToCopy import type { IAPIService, IControlService } from "@vrtmrz/livesync-commonlib/compat/services/base/IService"; export type ObsidianUIServiceDependencies = { appLifecycle: AppLifecycleService; - config: ConfigService; + config: IConfigService; replicator: ReplicatorService; APIService: IAPIService; control: IControlService; diff --git a/src/serviceFeatures/compatibilityReview.ts b/src/serviceFeatures/compatibilityReview.ts index b92aeec6..7c79f98f 100644 --- a/src/serviceFeatures/compatibilityReview.ts +++ b/src/serviceFeatures/compatibilityReview.ts @@ -44,17 +44,17 @@ export class CompatibilityReviewController { return this._initialised; } - private readAcknowledgedVersion(): string | null { + private async readAcknowledgedVersion(): Promise { const setting = this.core.services.setting; - const currentMarker = setting.getSmallConfig(DATABASE_COMPATIBILITY_VERSION_KEY); + const currentMarker = await this.core.services.config.getValue(DATABASE_COMPATIBILITY_VERSION_KEY, ""); if (currentMarker) return currentMarker; const legacyKey = legacyDatabaseCompatibilityVersionKey(this.core.services.vault.getVaultName()); - const legacyMarker = setting.getDeviceLocalConfig(legacyKey); + const legacyMarker = await setting.deviceValues.getValue(legacyKey, ""); if (!legacyMarker) return null; - setting.setSmallConfig(DATABASE_COMPATIBILITY_VERSION_KEY, legacyMarker); - setting.deleteDeviceLocalConfig(legacyKey); + await this.core.services.config.setValue(DATABASE_COMPATIBILITY_VERSION_KEY, legacyMarker); + await setting.deviceValues.delete(legacyKey); return legacyMarker; } @@ -75,7 +75,7 @@ export class CompatibilityReviewController { return true; } - const acknowledgedVersion = this.readAcknowledgedVersion(); + const acknowledgedVersion = await this.readAcknowledgedVersion(); const evaluation = evaluateCompatibilityPause({ acknowledgedVersion, currentVersion: this.currentVersion, @@ -85,7 +85,7 @@ export class CompatibilityReviewController { this.pause = evaluation.pause; if (evaluation.initialiseAcknowledgedVersion) { - setting.setSmallConfig(DATABASE_COMPATIBILITY_VERSION_KEY, `${this.currentVersion}`); + await this.core.services.config.setValue(DATABASE_COMPATIBILITY_VERSION_KEY, `${this.currentVersion}`); this._initialised = true; return true; } @@ -115,9 +115,9 @@ export class CompatibilityReviewController { throw error; } - setting.setSmallConfig(DATABASE_COMPATIBILITY_VERSION_KEY, `${this.currentVersion}`); + await this.core.services.config.setValue(DATABASE_COMPATIBILITY_VERSION_KEY, `${this.currentVersion}`); const legacyKey = legacyDatabaseCompatibilityVersionKey(this.core.services.vault.getVaultName()); - setting.deleteDeviceLocalConfig(legacyKey); + await setting.deviceValues.delete(legacyKey); this.pause = undefined; this.ui.clearReminder(); await this.core.services.control.applySettings(); diff --git a/src/serviceFeatures/compatibilityReview.unit.spec.ts b/src/serviceFeatures/compatibilityReview.unit.spec.ts index 760718cc..44a1b05b 100644 --- a/src/serviceFeatures/compatibilityReview.unit.spec.ts +++ b/src/serviceFeatures/compatibilityReview.unit.spec.ts @@ -33,13 +33,32 @@ function createFixture( } = {} ) { const local = new Map(); + const deviceValues = new Map(); if (options.marker !== undefined && options.marker !== null) { local.set(DATABASE_COMPATIBILITY_VERSION_KEY, options.marker); } const legacyKey = legacyDatabaseCompatibilityVersionKey("Test Vault"); if (options.legacyMarker !== undefined && options.legacyMarker !== null) { - local.set(legacyKey, options.legacyMarker); + deviceValues.set(legacyKey, options.legacyMarker); } + const config = { + getValue: vi.fn(async (key: string, defaultValue = "") => local.get(key) ?? defaultValue), + setValue: vi.fn(async (key: string, value: string) => { + local.set(key, value); + }), + delete: vi.fn(async (key: string) => { + local.delete(key); + }), + }; + const deviceValueStore = { + getValue: vi.fn(async (key: string, defaultValue = "") => deviceValues.get(key) ?? defaultValue), + setValue: vi.fn(async (key: string, value: string) => { + deviceValues.set(key, value); + }), + delete: vi.fn(async (key: string) => { + deviceValues.delete(key); + }), + }; const settings = { versionUpFlash: options.versionUpFlash ?? "", isConfigured: options.isConfigured ?? true, @@ -49,15 +68,13 @@ function createFixture( const setting = { currentSettings: () => settings, getSettingsMigrationState: () => migrationState(options.migration), - getSmallConfig: (key: string) => local.get(key) ?? "", - setSmallConfig: (key: string, value: string) => local.set(key, value), - getDeviceLocalConfig: (key: string) => local.get(key) ?? null, - deleteDeviceLocalConfig: (key: string) => local.delete(key), + deviceValues: deviceValueStore, saveSettingData, }; const core = { services: { setting, + config, vault: { getVaultName: () => "Test Vault" }, control: { applySettings }, }, @@ -69,7 +86,18 @@ function createFixture( clearReminder: vi.fn(), }; const controller = new CompatibilityReviewController(core, ui, 12); - return { controller, ui, local, legacyKey, settings, saveSettingData, applySettings }; + return { + controller, + ui, + local, + deviceValues, + config, + deviceValueStore, + legacyKey, + settings, + saveSettingData, + applySettings, + }; } describe("compatibility review controller", () => { @@ -196,10 +224,96 @@ describe("compatibility review controller", () => { await fixture.controller.initialise(); expect(fixture.local.get(DATABASE_COMPATIBILITY_VERSION_KEY)).toBe("11"); - expect(fixture.local.has(fixture.legacyKey)).toBe(false); + expect(fixture.deviceValues.has(fixture.legacyKey)).toBe(false); expect(fixture.controller.pendingPause).toBeDefined(); }); + it("saves the scoped marker before deleting the legacy device value", async () => { + const fixture = createFixture({ legacyMarker: "11" }); + let finishSave!: () => void; + const saving = new Promise((resolve) => { + finishSave = resolve; + }); + fixture.config.setValue.mockImplementation(async (key, value) => { + await saving; + fixture.local.set(key, value); + }); + + const initialising = fixture.controller.initialise(); + await Promise.resolve(); + await Promise.resolve(); + + expect(fixture.deviceValues.get(fixture.legacyKey)).toBe("11"); + expect(fixture.deviceValueStore.delete).not.toHaveBeenCalled(); + expect(fixture.controller.initialised).toBe(false); + + finishSave(); + await initialising; + + expect(fixture.local.get(DATABASE_COMPATIBILITY_VERSION_KEY)).toBe("11"); + expect(fixture.deviceValues.has(fixture.legacyKey)).toBe(false); + expect(fixture.controller.initialised).toBe(true); + }); + + it("does not treat a failed marker read as an absent value", async () => { + const fixture = createFixture({ legacyMarker: "11" }); + fixture.config.getValue.mockRejectedValueOnce(new Error("marker read failed")); + + await expect(fixture.controller.initialise()).rejects.toThrow("marker read failed"); + + expect(fixture.deviceValueStore.getValue).not.toHaveBeenCalled(); + expect(fixture.deviceValues.get(fixture.legacyKey)).toBe("11"); + expect(fixture.controller.initialised).toBe(false); + }); + + it("keeps the compatibility pause blocked until its async marker write succeeds", async () => { + const fixture = createFixture({ marker: "11" }); + vi.mocked(fixture.ui.showSummary).mockResolvedValue("resume"); + await fixture.controller.initialise(); + let finishSave!: () => void; + const saving = new Promise((resolve) => { + finishSave = resolve; + }); + fixture.config.setValue.mockImplementation(async (key, value) => { + await saving; + fixture.local.set(key, value); + }); + + const reviewing = fixture.controller.openReview(); + await Promise.resolve(); + await Promise.resolve(); + + expect(fixture.controller.pendingPause).toBeDefined(); + expect(fixture.local.get(DATABASE_COMPATIBILITY_VERSION_KEY)).toBe("11"); + expect(fixture.applySettings).not.toHaveBeenCalled(); + + finishSave(); + await reviewing; + + expect(fixture.local.get(DATABASE_COMPATIBILITY_VERSION_KEY)).toBe("12"); + expect(fixture.controller.pendingPause).toBeUndefined(); + expect(fixture.applySettings).toHaveBeenCalledOnce(); + }); + + it("keeps a storage-failed acknowledgement blocked and retryable", async () => { + const fixture = createFixture({ marker: "11" }); + vi.mocked(fixture.ui.showSummary).mockResolvedValue("resume"); + await fixture.controller.initialise(); + fixture.config.setValue.mockRejectedValueOnce(new Error("marker write failed")); + + await expect(fixture.controller.openReview()).rejects.toThrow("marker write failed"); + + expect(fixture.controller.pendingPause).toBeDefined(); + expect(fixture.local.get(DATABASE_COMPATIBILITY_VERSION_KEY)).toBe("11"); + expect(fixture.applySettings).not.toHaveBeenCalled(); + + await fixture.controller.openReview(); + + expect(fixture.controller.pendingPause).toBeUndefined(); + expect(fixture.local.get(DATABASE_COMPATIBILITY_VERSION_KEY)).toBe("12"); + expect(fixture.applySettings).toHaveBeenCalledOnce(); + }); + it("restores the runtime gate if persisting an acknowledgement fails", async () => { const fixture = createFixture({ marker: "11" }); vi.mocked(fixture.ui.showSummary).mockResolvedValue("resume"); diff --git a/src/serviceFeatures/redFlag.simpleFetch.ts b/src/serviceFeatures/redFlag.simpleFetch.ts index 4ce84e29..950e1618 100644 --- a/src/serviceFeatures/redFlag.simpleFetch.ts +++ b/src/serviceFeatures/redFlag.simpleFetch.ts @@ -63,28 +63,27 @@ function buildSimpleFetchResult(stage1: string, stage2?: string) { return undefined; } -function rememberSimpleFetchMode(host: NecessaryServices<"setting", never>, stage1: string, stage2?: string) { - host.services.setting.setSmallConfig(SIMPLE_FETCH_MODE_KEY, JSON.stringify({ stage1, stage2 })); +async function rememberSimpleFetchMode(host: NecessaryServices<"setting", never>, stage1: string, stage2?: string) { + await host.services.setting.config.setValue(SIMPLE_FETCH_MODE_KEY, JSON.stringify({ stage1, stage2 })); } -function getRememberedSimpleFetchMode(host: NecessaryServices<"setting", never>) { - const saved = host.services.setting.getSmallConfig(SIMPLE_FETCH_MODE_KEY); +async function getRememberedSimpleFetchMode(host: NecessaryServices<"setting", never>) { + const saved = await host.services.setting.config.getValue(SIMPLE_FETCH_MODE_KEY, ""); if (!saved) return undefined; + let remembered: ReturnType = undefined; try { const { stage1, stage2 } = JSON.parse(saved) as { stage1?: string; stage2?: string }; - if (stage1) { - const remembered = buildSimpleFetchResult(stage1, stage2); - if (remembered) return remembered; - } + if (stage1) remembered = buildSimpleFetchResult(stage1, stage2); } catch { // Clear below; the saved choice is optional and can be rebuilt by asking again. } - host.services.setting.deleteSmallConfig(SIMPLE_FETCH_MODE_KEY); + if (remembered) return remembered; + await host.services.setting.config.delete(SIMPLE_FETCH_MODE_KEY); return undefined; } -function clearRememberedSimpleFetchMode(host: NecessaryServices<"setting", never>) { - host.services.setting.deleteSmallConfig(SIMPLE_FETCH_MODE_KEY); +async function clearRememberedSimpleFetchMode(host: NecessaryServices<"setting", never>) { + await host.services.setting.config.delete(SIMPLE_FETCH_MODE_KEY); } async function confirmTranslatedChoice( @@ -104,7 +103,7 @@ async function confirmTranslatedChoice( export async function askSimpleFetchMode( host: NecessaryServices<"UI" | "setting", never> ): Promise<{ mode: string; options: Partial } | "cancelled" | "aborted"> { - const remembered = getRememberedSimpleFetchMode(host); + const remembered = await getRememberedSimpleFetchMode(host); if (remembered) return remembered; const msg = $msg( @@ -169,7 +168,7 @@ Firstly, how shall we handle the data retrieved from this remote source? if (stage2 === STAGE2_ABORT) { return "aborted"; } - rememberSimpleFetchMode(host, stage1, stage2); + await rememberSimpleFetchMode(host, stage1, stage2); return buildSimpleFetchResult(stage1, stage2)!; } @@ -198,7 +197,7 @@ Firstly, how shall we handle the data retrieved from this remote source? if (stage2 === STAGE2_ABORT) { return "aborted"; } - rememberSimpleFetchMode(host, stage1, stage2); + await rememberSimpleFetchMode(host, stage1, stage2); return buildSimpleFetchResult(stage1, stage2)!; } @@ -234,18 +233,18 @@ export async function askAndPerformFastSetupOnScheduledFetchAll( "Remediation mode is active, so the detailed fetch flow is used instead of Simple Fetch.", LOG_LEVEL_NOTICE ); - clearRememberedSimpleFetchMode(host); + await clearRememberedSimpleFetchMode(host); return undefined; } const result = await askSimpleFetchMode(host); if (result === "cancelled") { log("Fetch cancelled by user.", LOG_LEVEL_NOTICE); - clearRememberedSimpleFetchMode(host); + await clearRememberedSimpleFetchMode(host); return await cancelScheduledInitialisation(host, cleanupFlag); } if (result === "aborted") { log("Fetch exited by user.", LOG_LEVEL_NOTICE); - clearRememberedSimpleFetchMode(host); + await clearRememberedSimpleFetchMode(host); host.services.appLifecycle.performRestart(); return false; } @@ -256,7 +255,7 @@ export async function askAndPerformFastSetupOnScheduledFetchAll( const settings = host.services.setting.currentSettings(); if (!(await adjustSettingToRemoteIfNeeded(host, log, { preventFetchingConfig: false }, settings))) { log("Fetch initialisation cancelled by user.", LOG_LEVEL_NOTICE); - clearRememberedSimpleFetchMode(host); + await clearRememberedSimpleFetchMode(host); return await cancelScheduledInitialisation(host, cleanupFlag); } @@ -295,7 +294,7 @@ export async function askAndPerformFastSetupOnScheduledFetchAll( } await host.serviceModules.rebuilder.finishRebuild(); await cleanupFlag(); - clearRememberedSimpleFetchMode(host); + await clearRememberedSimpleFetchMode(host); log("Simple fetch and scan operation completed.", LOG_LEVEL_NOTICE); return true; }; diff --git a/src/serviceFeatures/redFlag.simpleFetch.unit.spec.ts b/src/serviceFeatures/redFlag.simpleFetch.unit.spec.ts index 73b3d9c0..bc583d8f 100644 --- a/src/serviceFeatures/redFlag.simpleFetch.unit.spec.ts +++ b/src/serviceFeatures/redFlag.simpleFetch.unit.spec.ts @@ -10,19 +10,26 @@ const KEEP_LOCAL = "Keep local files even if not on remote"; function createHost(savedChoice?: string) { const smallConfig = new Map(); if (savedChoice) smallConfig.set("simple-fetch-mode", savedChoice); + const config = { + getValue: vi.fn(async (key: string, defaultValue = "") => smallConfig.get(key) ?? defaultValue), + setValue: vi.fn(async (key: string, value: string) => { + smallConfig.set(key, value); + }), + delete: vi.fn(async (key: string) => { + smallConfig.delete(key); + }), + }; const confirmWithMessage = vi.fn<(title: string, message: string, buttons: string[], defaultAction: string) => Promise>(); const host = { services: { UI: { confirm: { confirmWithMessage } }, setting: { - getSmallConfig: (key: string) => smallConfig.get(key) ?? "", - setSmallConfig: (key: string, value: string) => smallConfig.set(key, value), - deleteSmallConfig: (key: string) => smallConfig.delete(key), + config, }, }, } as unknown as NecessaryServices<"UI" | "setting", never>; - return { host, confirmWithMessage, smallConfig }; + return { host, confirmWithMessage, smallConfig, config }; } afterEach(() => setLang("def")); @@ -50,6 +57,75 @@ describe("Fast Setup translated choices", () => { }); }); + it("waits for the saved choice before returning it to the fetch workflow", async () => { + const { host, confirmWithMessage, smallConfig, config } = createHost(); + let markSaveStarted!: () => void; + const saveStarted = new Promise((resolve) => { + markSaveStarted = resolve; + }); + let finishSave!: () => void; + const save = new Promise((resolve) => { + finishSave = resolve; + }); + config.setValue.mockImplementation(async (key, value) => { + markSaveStarted(); + await save; + smallConfig.set(key, value); + }); + confirmWithMessage.mockImplementation(async (_title, _message, buttons) => buttons[1]); + + const asking = askSimpleFetchMode(host); + await saveStarted; + + expect(smallConfig.has("simple-fetch-mode")).toBe(false); + expect(confirmWithMessage).toHaveBeenCalledTimes(2); + + finishSave(); + await expect(asking).resolves.toMatchObject({ mode: "remote-only" }); + expect(JSON.parse(smallConfig.get("simple-fetch-mode")!)).toEqual({ + stage1: REMOTE_WINS, + stage2: KEEP_LOCAL, + }); + }); + + it("propagates a stored-choice read failure instead of treating it as missing", async () => { + const { host, confirmWithMessage, config } = createHost(); + config.getValue.mockRejectedValueOnce(new Error("choice read failed")); + + await expect(askSimpleFetchMode(host)).rejects.toThrow("choice read failed"); + + expect(confirmWithMessage).not.toHaveBeenCalled(); + }); + + it("waits for an invalid optional choice to be deleted before asking again", async () => { + const { host, confirmWithMessage, smallConfig, config } = createHost("not-json"); + let markDeleteStarted!: () => void; + const deleteStarted = new Promise((resolve) => { + markDeleteStarted = resolve; + }); + let finishDelete!: () => void; + const deletion = new Promise((resolve) => { + finishDelete = resolve; + }); + config.delete.mockImplementation(async (key) => { + markDeleteStarted(); + await deletion; + smallConfig.delete(key); + }); + confirmWithMessage.mockImplementation(async (_title, _message, buttons) => buttons[3]); + + const asking = askSimpleFetchMode(host); + await deleteStarted; + + expect(confirmWithMessage).not.toHaveBeenCalled(); + expect(smallConfig.has("simple-fetch-mode")).toBe(true); + + finishDelete(); + await expect(asking).resolves.toBe("cancelled"); + expect(smallConfig.has("simple-fetch-mode")).toBe(false); + expect(confirmWithMessage).toHaveBeenCalledOnce(); + }); + it("restores choices saved by the previous English implementation while displaying Spanish", async () => { const savedChoice = JSON.stringify({ stage1: REMOTE_WINS, stage2: KEEP_LOCAL }); const { host, confirmWithMessage, smallConfig } = createHost(savedChoice); diff --git a/src/serviceFeatures/redFlag.unit.spec.ts b/src/serviceFeatures/redFlag.unit.spec.ts index 9c1a2bf8..b21cb99a 100644 --- a/src/serviceFeatures/redFlag.unit.spec.ts +++ b/src/serviceFeatures/redFlag.unit.spec.ts @@ -107,13 +107,15 @@ const createSettingServiceMock = () => { }), suspendAllSync: vi.fn(() => Promise.resolve()), suspendExtraSync: vi.fn(() => Promise.resolve()), - getSmallConfig: vi.fn((key: string) => smallConfig.get(key) ?? ""), - setSmallConfig: vi.fn((key: string, value: string) => { - smallConfig.set(key, value); - }), - deleteSmallConfig: vi.fn((key: string) => { - smallConfig.delete(key); - }), + config: { + getValue: vi.fn(async (key: string, defaultValue = "") => smallConfig.get(key) ?? defaultValue), + setValue: vi.fn(async (key: string, value: string) => { + smallConfig.set(key, value); + }), + delete: vi.fn(async (key: string) => { + smallConfig.delete(key); + }), + }, }; }; @@ -785,7 +787,7 @@ describe("Red Flag Feature", () => { ); expect(host.mocks.ui.confirm.confirmWithMessage).not.toHaveBeenCalled(); - expect(host.mocks.setting.deleteSmallConfig).toHaveBeenCalledWith("simple-fetch-mode"); + expect(host.mocks.setting.config.delete).toHaveBeenCalledWith("simple-fetch-mode"); expect(cleanupFlag).not.toHaveBeenCalled(); }); @@ -830,7 +832,7 @@ describe("Red Flag Feature", () => { }); expect(host.mocks.rebuilder.finishRebuild).not.toHaveBeenCalled(); expect(cleanupFlag).not.toHaveBeenCalled(); - expect(host.mocks.setting.deleteSmallConfig).not.toHaveBeenCalledWith("simple-fetch-mode"); + expect(host.mocks.setting.config.delete).not.toHaveBeenCalledWith("simple-fetch-mode"); }); it("re-suspends both reflection directions when finalisation fails after releasing them", async () => { @@ -892,7 +894,7 @@ describe("Red Flag Feature", () => { await askAndPerformFastSetupOnScheduledFetchAll(host as any, log, cleanupFlag); - expect(host.mocks.setting.deleteSmallConfig).toHaveBeenCalledWith("simple-fetch-mode"); + expect(host.mocks.setting.config.delete).toHaveBeenCalledWith("simple-fetch-mode"); }); it("should return false and cleanup when quick flow is cancelled", async () => { diff --git a/src/serviceFeatures/useReviewHarness.ts b/src/serviceFeatures/useReviewHarness.ts index 99e2cd5a..64efb03e 100644 --- a/src/serviceFeatures/useReviewHarness.ts +++ b/src/serviceFeatures/useReviewHarness.ts @@ -69,9 +69,9 @@ export function useReviewHarness( await createFileContentHashBenchmarkOperations(activeWindow.performance), activeWindow.performance ), - readContinuation: () => services.setting.getSmallConfig(REVIEW_HARNESS_STATE_KEY), - writeContinuation: (value) => services.setting.setSmallConfig(REVIEW_HARNESS_STATE_KEY, value), - deleteContinuation: () => services.setting.deleteSmallConfig(REVIEW_HARNESS_STATE_KEY), + readContinuation: () => services.config.getValue(REVIEW_HARNESS_STATE_KEY, ""), + writeContinuation: (value) => services.config.setValue(REVIEW_HARNESS_STATE_KEY, value), + deleteContinuation: () => services.config.delete(REVIEW_HARNESS_STATE_KEY), restart: () => services.appLifecycle.performRestart(), reportError: (error) => services.API.addLog(error, LOG_LEVEL_NOTICE), copyText: async (value) => { @@ -94,14 +94,14 @@ export function useReviewHarness( let registered = false; let openAfterLayout = false; - services.appLifecycle.onSettingLoaded.addHandler(() => { + services.appLifecycle.onSettingLoaded.addHandler(async () => { if (!continuationConsumed) { + await controller.consumeContinuation(); continuationConsumed = true; - controller.consumeContinuation(); } const snapshot = controller.snapshot(); openAfterLayout = snapshot.resumedRequestId !== null || snapshot.continuationError !== null; - if (!services.setting.currentSettings().enableDebugTools || registered) return Promise.resolve(true); + if (!services.setting.currentSettings().enableDebugTools || registered) return true; registered = true; services.API.registerWindow( @@ -115,7 +115,7 @@ export function useReviewHarness( void services.API.showWindow(VIEW_TYPE_REVIEW_HARNESS); }, }); - return Promise.resolve(true); + return true; }); services.appLifecycle.onLayoutReady.addHandler(() => { diff --git a/src/serviceFeatures/useReviewHarness.unit.spec.ts b/src/serviceFeatures/useReviewHarness.unit.spec.ts index 35281081..c73070a7 100644 --- a/src/serviceFeatures/useReviewHarness.unit.spec.ts +++ b/src/serviceFeatures/useReviewHarness.unit.spec.ts @@ -31,6 +31,15 @@ function createFixture(options: { enableDebugTools?: boolean; continuation?: str const layoutReadyHandlers: Array<() => Promise> = []; const local = new Map(); if (options.continuation) local.set(REVIEW_HARNESS_STATE_KEY, options.continuation); + const config = { + getValue: vi.fn(async (key: string, defaultValue = "") => local.get(key) ?? defaultValue), + setValue: vi.fn(async (key: string, value: string) => { + local.set(key, value); + }), + delete: vi.fn(async (key: string) => { + local.delete(key); + }), + }; const settings = { enableDebugTools: options.enableDebugTools ?? true, liveSync: false, @@ -63,10 +72,8 @@ function createFixture(options: { enableDebugTools?: boolean; continuation?: str requiresSyncReview: true, reviewReasons: [], }), - getSmallConfig: (key: string) => local.get(key) ?? "", - setSmallConfig: (key: string, value: string) => local.set(key, value), - deleteSmallConfig: (key: string) => local.delete(key), }, + config, appLifecycle: { onSettingLoaded: { addHandler: vi.fn((handler: () => Promise) => settingLoadedHandlers.push(handler)), @@ -106,6 +113,7 @@ function createFixture(options: { enableDebugTools?: boolean; continuation?: str api, settings, local, + config, settingLoadedHandlers, layoutReadyHandlers, compatibilityReview, @@ -144,6 +152,74 @@ describe("Review Harness composition", () => { expect(fixture.api.showWindow).toHaveBeenCalledWith(VIEW_TYPE_REVIEW_HARNESS); }); + it("holds the setting-loaded lifecycle until continuation deletion has completed", async () => { + const requestedAt = "2026-07-18T11:59:00.000Z"; + const continuation = JSON.stringify({ + formatVersion: 1, + requestId: `compatibility-review-${requestedAt}`, + scenarioId: "compatibility-review", + stage: "awaiting-restart", + requestedAt, + }); + const fixture = createFixture({ continuation }); + let markDeleteStarted!: () => void; + const deleteStarted = new Promise((resolve) => { + markDeleteStarted = resolve; + }); + let finishDelete!: () => void; + const deleting = new Promise((resolve) => { + finishDelete = resolve; + }); + fixture.config.delete.mockImplementation(async (key) => { + markDeleteStarted(); + await deleting; + fixture.local.delete(key); + }); + + const loading = fixture.settingLoadedHandlers[0](); + await deleteStarted; + + expect(fixture.local.has(REVIEW_HARNESS_STATE_KEY)).toBe(true); + expect(fixture.controller.snapshot().resumedRequestId).toBeNull(); + expect(fixture.api.registerWindow).not.toHaveBeenCalled(); + + finishDelete(); + await loading; + + expect(fixture.local.has(REVIEW_HARNESS_STATE_KEY)).toBe(false); + expect(fixture.controller.snapshot().resumedRequestId).toBe(`compatibility-review-${requestedAt}`); + expect(fixture.api.registerWindow).toHaveBeenCalledOnce(); + }); + + it("can retry one-shot continuation consumption after a deletion failure", async () => { + const requestedAt = "2026-07-18T11:59:00.000Z"; + const continuation = JSON.stringify({ + formatVersion: 1, + requestId: `compatibility-review-${requestedAt}`, + scenarioId: "compatibility-review", + stage: "awaiting-restart", + requestedAt, + }); + const fixture = createFixture({ continuation }); + fixture.config.delete + .mockRejectedValueOnce(new Error("continuation delete failed")) + .mockImplementation(async (key) => { + fixture.local.delete(key); + }); + + await expect(fixture.settingLoadedHandlers[0]()).rejects.toThrow("continuation delete failed"); + + expect(fixture.local.has(REVIEW_HARNESS_STATE_KEY)).toBe(true); + expect(fixture.controller.snapshot().resumedRequestId).toBeNull(); + expect(fixture.api.registerWindow).not.toHaveBeenCalled(); + + await fixture.settingLoadedHandlers[0](); + + expect(fixture.local.has(REVIEW_HARNESS_STATE_KEY)).toBe(false); + expect(fixture.controller.snapshot().resumedRequestId).toBe(`compatibility-review-${requestedAt}`); + expect(fixture.api.registerWindow).toHaveBeenCalledOnce(); + }); + it("uses the actual compatibility controller as the guided review boundary", async () => { const fixture = createFixture(); diff --git a/test/apps/webapp/WebAppRuntime.unit.spec.ts b/test/apps/webapp/WebAppRuntime.unit.spec.ts index 34d2cffa..e5aae6c3 100644 --- a/test/apps/webapp/WebAppRuntime.unit.spec.ts +++ b/test/apps/webapp/WebAppRuntime.unit.spec.ts @@ -1,3 +1,4 @@ +import { MemoryValueStore } from "@vrtmrz/value-store"; import { LOG_LEVEL_INFO, type ObsidianLiveSyncSettings } from "@vrtmrz/livesync-commonlib/compat/common/types"; import { beforeEach, describe, expect, it, vi } from "vitest"; import type { LiveSyncBrowserServiceHubOptions } from "@/apps/browser/createLiveSyncBrowserServiceHub"; @@ -168,7 +169,7 @@ describe("WebAppRuntime lifecycle", () => { it("owns core start, directory scan, status reporting, and shutdown", async () => { const reportStatus = vi.fn(); - const runtime = new WebAppRuntime(createRootHandle(), { reportStatus }); + const runtime = new WebAppRuntime(createRootHandle(), { reportStatus, deviceValues: new MemoryValueStore() }); await runtime.start(); @@ -193,7 +194,7 @@ describe("WebAppRuntime lifecycle", () => { }); it("finalises application readiness after importing local files for optional P2P", async () => { - const runtime = new WebAppRuntime(createRootHandle()); + const runtime = new WebAppRuntime(createRootHandle(), { deviceValues: new MemoryValueStore() }); await runtime.start(); vi.clearAllMocks(); runtimeMocks.currentSettings.mockReturnValue(unconfiguredSettings); @@ -239,7 +240,7 @@ describe("WebAppRuntime lifecycle", () => { }); it("does not publish readiness after a partial local-file import", async () => { - const runtime = new WebAppRuntime(createRootHandle()); + const runtime = new WebAppRuntime(createRootHandle(), { deviceValues: new MemoryValueStore() }); await runtime.start(); vi.clearAllMocks(); runtimeMocks.collectFilesOnStorage.mockResolvedValue({ @@ -269,7 +270,7 @@ describe("WebAppRuntime lifecycle", () => { }); it("does not publish readiness when database initialisation rejects manual preparation", async () => { - const runtime = new WebAppRuntime(createRootHandle()); + const runtime = new WebAppRuntime(createRootHandle(), { deviceValues: new MemoryValueStore() }); await runtime.start(); vi.clearAllMocks(); runtimeMocks.onDatabaseInitialised.mockResolvedValue(false); @@ -282,7 +283,7 @@ describe("WebAppRuntime lifecycle", () => { }); it("does not publish readiness when pending file events cannot be committed", async () => { - const runtime = new WebAppRuntime(createRootHandle()); + const runtime = new WebAppRuntime(createRootHandle(), { deviceValues: new MemoryValueStore() }); await runtime.start(); vi.clearAllMocks(); runtimeMocks.commitPendingFileEvents.mockResolvedValue(false); @@ -295,7 +296,7 @@ describe("WebAppRuntime lifecycle", () => { }); it("does not repeat readiness finalisation after a successful manual scan", async () => { - const runtime = new WebAppRuntime(createRootHandle()); + const runtime = new WebAppRuntime(createRootHandle(), { deviceValues: new MemoryValueStore() }); await runtime.start(); vi.clearAllMocks(); @@ -311,7 +312,7 @@ describe("WebAppRuntime lifecycle", () => { it("rejects a failed core start after cleaning up the partial runtime", async () => { runtimeMocks.onLoad.mockResolvedValue(false); const reportStatus = vi.fn(); - const runtime = new WebAppRuntime(createRootHandle(), { reportStatus }); + const runtime = new WebAppRuntime(createRootHandle(), { reportStatus, deviceValues: new MemoryValueStore() }); await expect(runtime.start()).rejects.toThrow("Failed to initialise LiveSync"); @@ -325,7 +326,7 @@ describe("WebAppRuntime lifecycle", () => { it("shuts down once before scheduling a host reload", async () => { const scheduleReload = vi.fn(); - const runtime = new WebAppRuntime(createRootHandle(), { scheduleReload }); + const runtime = new WebAppRuntime(createRootHandle(), { scheduleReload, deviceValues: new MemoryValueStore() }); await runtime.start(); runtimeMocks.options?.restart?.schedule(); diff --git a/test/apps/webpeer/WebPeerRuntime.unit.spec.ts b/test/apps/webpeer/WebPeerRuntime.unit.spec.ts index 2d8f678c..693b1e3d 100644 --- a/test/apps/webpeer/WebPeerRuntime.unit.spec.ts +++ b/test/apps/webpeer/WebPeerRuntime.unit.spec.ts @@ -1,3 +1,4 @@ +import { MemoryValueStore } from "@vrtmrz/value-store"; import { createServiceContext } from "@vrtmrz/livesync-commonlib/context"; import { DEFAULT_SETTINGS, SETTING_KEY_P2P_DEVICE_NAME } from "@vrtmrz/livesync-commonlib/compat/common/types"; import type { SimpleStore } from "octagonal-wheels/databases/SimpleStoreBase"; @@ -27,6 +28,7 @@ describe("WebPeer runtime composition", () => { translate: (key) => `webpeer:${key}`, }); const runtime = new WebPeerRuntime({ + deviceValues: new MemoryValueStore(), context, store: createMemoryStore(), }); @@ -43,6 +45,7 @@ describe("WebPeer runtime composition", () => { it("loads settings and opens the local database before announcing layout readiness", async () => { const runtime = new WebPeerRuntime({ + deviceValues: new MemoryValueStore(), store: createMemoryStore(), }); const loadSettings = vi.spyOn(runtime.services.setting, "loadSettings").mockResolvedValue(undefined); @@ -68,6 +71,7 @@ describe("WebPeer runtime composition", () => { vi.useFakeTimers(); try { const runtime = new WebPeerRuntime({ + deviceValues: new MemoryValueStore(), store: createMemoryStore(), }); vi.spyOn(runtime.services.setting, "loadSettings").mockResolvedValue(undefined); @@ -77,9 +81,7 @@ describe("WebPeer runtime composition", () => { P2P_Enabled: true, }); vi.spyOn(runtime.services.database, "openDatabase").mockResolvedValue(true); - const onResumed = vi - .spyOn(runtime.services.appLifecycle, "onResumed") - .mockResolvedValue(true); + const onResumed = vi.spyOn(runtime.services.appLifecycle, "onResumed").mockResolvedValue(true); const open = vi.spyOn(runtime.p2p.transportLifecycle, "connect").mockResolvedValue(undefined); await runtime.start(); @@ -94,9 +96,11 @@ describe("WebPeer runtime composition", () => { it("rejects start when the browser-local database cannot be opened", async () => { const runtime = new WebPeerRuntime({ + deviceValues: new MemoryValueStore(), store: createMemoryStore(), }); vi.spyOn(runtime.services.setting, "loadSettings").mockResolvedValue(undefined); + vi.spyOn(runtime.services.setting, "currentSettings").mockReturnValue({ ...DEFAULT_SETTINGS }); vi.spyOn(runtime.services.database, "openDatabase").mockResolvedValue(false); await expect(runtime.start()).rejects.toThrow("WebPeer local database could not be opened"); @@ -104,6 +108,7 @@ describe("WebPeer runtime composition", () => { it("isolates a specialised runtime and applies its device name before opening the database", async () => { const runtime = new WebPeerRuntime({ + deviceValues: new MemoryValueStore(), store: createMemoryStore(), deviceName: " p2p-check-browser-desktop-abc ", systemVaultName: "p2p-check-vault", @@ -114,15 +119,39 @@ describe("WebPeer runtime composition", () => { P2P_AutoStart: false, P2P_Enabled: false, }); - const setSmallConfig = vi.spyOn(runtime.services.config, "setSmallConfig").mockImplementation(() => undefined); + let finishWrite!: () => void; + const setValue = vi.spyOn(runtime.services.config, "setValue").mockImplementation( + () => + new Promise((resolve) => { + finishWrite = resolve; + }) + ); const openDatabase = vi.spyOn(runtime.services.database, "openDatabase").mockImplementation(async () => { - expect(setSmallConfig).toHaveBeenCalledWith(SETTING_KEY_P2P_DEVICE_NAME, "p2p-check-browser-desktop-abc"); + expect(setValue).toHaveBeenCalledWith(SETTING_KEY_P2P_DEVICE_NAME, "p2p-check-browser-desktop-abc"); return true; }); - await runtime.start(); + const starting = runtime.start(); + await vi.waitFor(() => expect(setValue).toHaveBeenCalledOnce()); + expect(openDatabase).not.toHaveBeenCalled(); + finishWrite(); + await starting; expect(runtime.services.API.getSystemVaultName()).toBe("p2p-check-vault"); expect(openDatabase).toHaveBeenCalledOnce(); }); + + it("reads the current device name after the pane updates the store", async () => { + const runtime = new WebPeerRuntime({ + deviceValues: new MemoryValueStore(), + store: createMemoryStore(), + systemVaultName: "webpeer-vault", + }); + vi.spyOn(runtime.services.setting, "currentSettings").mockReturnValue({ ...DEFAULT_SETTINGS }); + expect(await runtime.getDeviceName()).toBe("webpeer-vault"); + await runtime.services.config.setValue(SETTING_KEY_P2P_DEVICE_NAME, "first-name"); + expect(await runtime.getDeviceName()).toBe("first-name"); + await runtime.services.config.setValue(SETTING_KEY_P2P_DEVICE_NAME, "renamed-device"); + expect(await runtime.getDeviceName()).toBe("renamed-device"); + }); }); diff --git a/test/contracts/serviceContext.ts b/test/contracts/serviceContext.ts index ddfb6ce4..ec712e09 100644 --- a/test/contracts/serviceContext.ts +++ b/test/contracts/serviceContext.ts @@ -17,13 +17,12 @@ export const SERVICE_CONTEXT_MEMBERS = [ "vault", "test", "UI", - "config", "keyValueDB", "control", -] as const satisfies readonly Exclude[]; +] as const satisfies readonly Exclude[]; export type ServiceContextMember = (typeof SERVICE_CONTEXT_MEMBERS)[number]; -type MissingServiceContextMember = Exclude, ServiceContextMember>; +type MissingServiceContextMember = Exclude, ServiceContextMember>; const serviceContextMembersAreExhaustive: [MissingServiceContextMember] extends [never] ? true : never = true; void serviceContextMembersAreExhaustive; diff --git a/test/e2e-obsidian/runner/liveSyncWorkflow.ts b/test/e2e-obsidian/runner/liveSyncWorkflow.ts index cde2d049..e256fc10 100644 --- a/test/e2e-obsidian/runner/liveSyncWorkflow.ts +++ b/test/e2e-obsidian/runner/liveSyncWorkflow.ts @@ -111,7 +111,7 @@ export async function readE2eCompatibilityMarker( return await evalObsidianJson( cliBinary, [ - "(()=>{", + "(async()=>{", "const core=app.plugins.plugins['obsidian-livesync'].core;", "const setting=core.services.setting;", "const settings=setting.currentSettings();", @@ -122,7 +122,7 @@ export async function readE2eCompatibilityMarker( "return JSON.stringify({", "vaultName,additionalSuffix,expectedStorageKey,", "rawStorageValue:localStorage.getItem(expectedStorageKey),", - "serviceValue:setting.getSmallConfig(markerKey),", + "serviceValue:await setting.config.getValue(markerKey,''),", "versionUpFlash:settings.versionUpFlash,", "});", "})()", diff --git a/test/e2e-obsidian/runner/upgradeWorkflow.ts b/test/e2e-obsidian/runner/upgradeWorkflow.ts index 76b6c7aa..1af10d32 100644 --- a/test/e2e-obsidian/runner/upgradeWorkflow.ts +++ b/test/e2e-obsidian/runner/upgradeWorkflow.ts @@ -288,7 +288,7 @@ export async function readRuntimeUpgradeState( "localDatabaseDocumentCount:databaseInfo.doc_count,", "nodeId:nodeInfo?.nodeid??replicator?.nodeid??'',", "legacyCompatibilityMarker:localStorage.getItem(`obsidian-live-sync-ver${vaultName}`),", - "compatibilityMarker:setting.getSmallConfig('database-compatibility-version')??'',", + "compatibilityMarker:await (setting.config?.getValue?.('database-compatibility-version','')??Promise.resolve(setting.getSmallConfig?.('database-compatibility-version')??'')),", "compatibilityStorageEntries:Object.fromEntries(Array.from({length:localStorage.length},(_,index)=>localStorage.key(index))", ".filter((key)=>key!==null)", ".filter(key=>key.startsWith('obsidian-live-sync-ver')||key.endsWith('-database-compatibility-version'))", @@ -335,7 +335,7 @@ export async function readRuntimeSettingsUpgradeState( "const migrationState=setting.getSettingsMigrationState?.();", "return JSON.stringify({", "pluginVersion:app.plugins.manifests['obsidian-livesync']?.version??'unknown',", - "compatibilityMarker:setting.getSmallConfig?.('database-compatibility-version')??'',", + "compatibilityMarker:await (setting.config?.getValue?.('database-compatibility-version','')??Promise.resolve(setting.getSmallConfig?.('database-compatibility-version')??'')),", "migrationState,", "settings:{", "isConfigured:settings.isConfigured,settingVersion:settings.settingVersion,", diff --git a/test/e2e-obsidian/scripts/dialog-mounts.ts b/test/e2e-obsidian/scripts/dialog-mounts.ts index 5f3e6307..2456d9cd 100644 --- a/test/e2e-obsidian/scripts/dialog-mounts.ts +++ b/test/e2e-obsidian/scripts/dialog-mounts.ts @@ -1,7 +1,9 @@ import { readFile } from "node:fs/promises"; import { join } from "node:path"; +import type { Page } from "playwright"; import { decodeSettingsFromSetupURI } from "@vrtmrz/livesync-commonlib/setup-uri"; import { decryptString } from "@vrtmrz/livesync-commonlib/compat/encryption/stringEncryption"; +import { SETTING_KEY_P2P_DEVICE_NAME } from "@vrtmrz/livesync-commonlib/compat/common/types"; import { $msg } from "../../../src/common/translation.ts"; import { discoverObsidianCli, requireObsidianBinary } from "../runner/environment.ts"; import { createE2eCouchDbPluginData, waitForLiveSyncCoreReady } from "../runner/liveSyncWorkflow.ts"; @@ -13,18 +15,34 @@ import { captureObsidianPage, obsidianRemoteDebuggingPort, openLiveSyncSettings, - withObsidianPage, + withObsidianPage as withObsidianSessionPage, } from "../runner/ui.ts"; import { createTemporaryVault } from "../runner/vault.ts"; const dialogRunStateKey = "__livesyncE2EDialogMount"; +const savedP2PDeviceName = "e2e-p2p-saved-device"; const repairRunStateKey = "__livesyncE2ETroubleshootingRepair"; const uiTimeoutMs = Number(process.env.E2E_OBSIDIAN_DIALOG_TIMEOUT_MS ?? 10000); +async function withObsidianPage(port: number, operation: (page: Page) => Promise): Promise { + return await withObsidianSessionPage(port, async (page) => { + for (const candidate of page.context().pages()) { + const hasPlugin = await candidate + .evaluate( + () => (globalThis as ObsidianTestGlobal).app?.plugins?.plugins["obsidian-livesync"] !== undefined + ) + .catch(() => false); + if (hasPlugin) return await operation(candidate); + } + throw new Error("The LiveSync Vault renderer is unavailable."); + }); +} + type DialogueMode = "desktop" | "mobile"; type DialogueRunState = { done: boolean; + deviceNameRead?: boolean; error?: string; expected?: unknown; kind: string; @@ -46,6 +64,14 @@ type LiveSyncTestPlugin = { createAllChunks(force: boolean): Promise; }; modules: SetupManagerHandle[]; + services: { + setting: { + config: { + getValue(key: string, defaultValue?: string): Promise; + setValue(key: string, value: string): Promise; + }; + }; + }; settings: Record; }; }; @@ -91,39 +117,63 @@ async function openRemoteSelectionDialogue(): Promise { async function openP2PRemoteSelectionDialogueForInspection(): Promise { await withObsidianPage(obsidianRemoteDebuggingPort(), async (page) => { - await page.evaluate((stateKey) => { - const plugin = (globalThis as ObsidianTestGlobal).app?.plugins?.plugins["obsidian-livesync"]; - if (plugin === undefined) throw new Error("Self-hosted LiveSync is not loaded"); - const manager = plugin.core.modules.find((module) => module.constructor.name === "SetupManager"); - if ( - typeof manager?.onSelectServer !== "function" || - typeof manager.onConfirmApplySettingsFromWizard !== "function" - ) { - throw new Error("Could not find the P2P setup workflow"); + await page.evaluate( + async ({ stateKey, deviceNameKey, deviceName }) => { + const plugin = (globalThis as ObsidianTestGlobal).app?.plugins?.plugins["obsidian-livesync"]; + if (plugin === undefined) throw new Error("Self-hosted LiveSync is not loaded"); + const initialPeerName = plugin.core.settings.P2P_DevicePeerName; + if (typeof initialPeerName === "string" && initialPeerName.trim() !== "") { + throw new Error("The P2P setup fixture unexpectedly has an initial device name."); + } + const manager = plugin.core.modules.find((module) => module.constructor.name === "SetupManager"); + if ( + typeof manager?.onSelectServer !== "function" || + typeof manager.onConfirmApplySettingsFromWizard !== "function" + ) { + throw new Error("Could not find the P2P setup workflow"); + } + const config = plugin.core.services.setting.config; + await config.setValue(deviceNameKey, deviceName); + const originalGetValue = config.getValue; + const originalConfirm = manager.onConfirmApplySettingsFromWizard; + const state: DialogueRunState = { + kind: "p2p-compatibility-settings", + done: false, + deviceNameRead: false, + }; + (globalThis as unknown as Record)[stateKey] = state; + config.getValue = async (key, defaultValue) => { + const value = await originalGetValue.call(config, key, defaultValue); + if (key === deviceNameKey && value === deviceName) state.deviceNameRead = true; + return value; + }; + manager.onConfirmApplySettingsFromWizard = async (settings: unknown) => { + state.expected = settings; + return true; + }; + void manager + .onSelectServer(plugin.core.settings, "unknown") + .then( + (result) => { + state.result = result; + state.done = true; + }, + (error: unknown) => { + state.error = error instanceof Error ? error.message : String(error); + state.done = true; + } + ) + .finally(() => { + manager.onConfirmApplySettingsFromWizard = originalConfirm; + config.getValue = originalGetValue; + }); + }, + { + stateKey: dialogRunStateKey, + deviceNameKey: SETTING_KEY_P2P_DEVICE_NAME, + deviceName: savedP2PDeviceName, } - const originalConfirm = manager.onConfirmApplySettingsFromWizard; - const state: DialogueRunState = { kind: "p2p-compatibility-settings", done: false }; - (globalThis as unknown as Record)[stateKey] = state; - manager.onConfirmApplySettingsFromWizard = async (settings: unknown) => { - state.expected = settings; - return true; - }; - void manager - .onSelectServer(plugin.core.settings, "unknown") - .then( - (result) => { - state.result = result; - state.done = true; - }, - (error: unknown) => { - state.error = error instanceof Error ? error.message : String(error); - state.done = true; - } - ) - .finally(() => { - manager.onConfirmApplySettingsFromWizard = originalConfirm; - }); - }, dialogRunStateKey); + ); }); } @@ -388,7 +438,7 @@ async function verifyRemoteSelectionDialogue(mode: DialogueMode): Promise !element.isConnected, previousDialogue, { timeout: uiTimeoutMs }); await modal.waitFor({ state: "visible", timeout: uiTimeoutMs }); } @@ -495,9 +545,21 @@ async function verifyP2PCompatibilitySettingsDialogue(): Promise { has: page.locator(".modal-title").filter({ hasText: "P2P Configuration" }), }); await modal.waitFor({ state: "visible", timeout: uiTimeoutMs }); + const peerName = modal.locator('input[name="p2p-device-peer-id"]'); + await peerName.waitFor({ state: "visible", timeout: uiTimeoutMs }); + await page.waitForFunction( + (expectedName) => { + const input = document.querySelector('input[name="p2p-device-peer-id"]'); + return input instanceof HTMLInputElement && !input.disabled && input.value === expectedName; + }, + savedP2PDeviceName, + { timeout: uiTimeoutMs } + ); + if ((await peerName.inputValue()) !== savedP2PDeviceName) { + throw new Error("The saved P2P device name was not loaded before submitting the setup dialogue."); + } await modal.locator('input[name="p2p-room-id"]').fill("e2e-p2p-compatibility"); await modal.locator('input[name="p2p-password"]').fill("e2e-passphrase"); - await modal.locator('input[name="p2p-device-peer-id"]').fill("e2e-device"); const compatibility = modal.locator("details").filter({ has: page.locator("summary").filter({ hasText: "Connection compatibility" }), @@ -565,8 +627,7 @@ async function verifyP2PCompatibilitySettingsDialogue(): Promise { { timeout: uiTimeoutMs } ); const resetNotice = compatibility.locator(".sls-info-note-notice").filter({ - hasText: - "TURN relay only requires TURN configuration. Connection path has been restored to Automatic.", + hasText: "TURN relay only requires TURN configuration. Connection path has been restored to Automatic.", }); await resetNotice.waitFor({ state: "visible", timeout: uiTimeoutMs }); await resetNotice @@ -594,10 +655,16 @@ async function verifyP2PCompatibilitySettingsDialogue(): Promise { }); const state = await assertDialogueRunCompleted(); + if (state.deviceNameRead !== true) { + throw new Error("The saved P2P device name was not read through setting.config."); + } if (state.result !== true || typeof state.expected !== "object" || state.expected === null) { throw new Error("The P2P compatibility settings were not passed to the setup workflow."); } const settings = state.expected as Record; + if (settings.P2P_DevicePeerName !== savedP2PDeviceName) { + throw new Error("The saved P2P device name was not passed through the setup workflow."); + } if (settings.P2P_maxWirePayloadBytes !== 800 || settings.P2P_connectionPath !== "relay") { throw new Error( `The P2P compatibility choices did not round-trip through the setup dialogue: ${JSON.stringify({ @@ -672,15 +739,14 @@ async function verifyGenerateSetupUriDialogue(mode: DialogueMode): Promise - (globalThis as ObsidianTestGlobal).app?.commands?.executeCommandById(commandId) === true, + (commandId) => (globalThis as ObsidianTestGlobal).app?.commands?.executeCommandById(commandId) === true, "obsidian-livesync:livesync-copysetupuri" ); }); @@ -714,7 +780,8 @@ async function verifyGenerateSetupUriDialogue(mode: DialogueMode): Promise { if (!(await toggle.evaluate((element) => element.classList.contains("is-enabled")))) { throw new Error("The automatic compatible-setting policy was displayed as disabled while still undefined."); } + await settingsNavigator.close(); }); } @@ -1094,9 +1162,10 @@ async function verifyHatchSurfacesAndSafeActions(): Promise { ); await withObsidianPage(obsidianRemoteDebuggingPort(), async (page) => { - const liveSyncSettings = page.locator(".sls-setting"); + const settingsNavigator = await openLiveSyncSettings(page, uiTimeoutMs); + const liveSyncSettings = await settingsNavigator.openPage("Hatch"); const logSetting = liveSyncSettings.locator(".setting-item").filter({ - has: page.getByText("Write logs into the file", { exact: true }), + has: settingsNavigator.page.getByText("Write logs into the file", { exact: true }), }); await logSetting.locator(".checkbox-container").click({ timeout: uiTimeoutMs }); await page.waitForFunction( @@ -1130,10 +1199,10 @@ async function verifyHatchSurfacesAndSafeActions(): Promise { // Saving a toggle refreshes the settings pane. Resolve the visible control again so the // second action does not target the detached pre-save element. - const refreshedLogToggle = page - .locator(".sls-setting:visible .setting-item:visible") + const refreshedLogToggle = liveSyncSettings + .locator(".setting-item:visible") .filter({ - has: page.getByText("Write logs into the file", { exact: true }), + has: settingsNavigator.page.getByText("Write logs into the file", { exact: true }), }) .locator(".checkbox-container:visible") .last(); @@ -1179,13 +1248,9 @@ async function verifyHatchSurfacesAndSafeActions(): Promise { } }; }, repairRunStateKey); - await page - .locator(".sls-setting:visible") - .last() - .getByRole("button", { name: "Recreate current chunks", exact: true }) - .click({ - timeout: uiTimeoutMs, - }); + await liveSyncSettings.getByRole("button", { name: "Recreate current chunks", exact: true }).click({ + timeout: uiTimeoutMs, + }); await page.waitForFunction( (stateKey) => (globalThis as unknown as Record)[stateKey]?.done === true, @@ -1200,13 +1265,9 @@ async function verifyHatchSurfacesAndSafeActions(): Promise { throw new Error(`Recreate missing chunks failed: ${repairState.error}`); } - await page - .locator(".sls-setting:visible") - .last() - .getByRole("button", { name: "Begin inspection", exact: true }) - .click({ - timeout: uiTimeoutMs, - }); + await liveSyncSettings.getByRole("button", { name: "Begin inspection", exact: true }).click({ + timeout: uiTimeoutMs, + }); await page .locator(".notice") .filter({ hasText: /^done$/u }) @@ -1214,6 +1275,7 @@ async function verifyHatchSurfacesAndSafeActions(): Promise { state: "visible", timeout: uiTimeoutMs, }); + await settingsNavigator.close(); }); return screenshotPath; diff --git a/test/e2e-obsidian/scripts/review-harness.ts b/test/e2e-obsidian/scripts/review-harness.ts index 49e22c67..64631db9 100644 --- a/test/e2e-obsidian/scripts/review-harness.ts +++ b/test/e2e-obsidian/scripts/review-harness.ts @@ -372,13 +372,21 @@ async function restartAndResumeHarness(): Promise { await harness .locator('[data-testid="review-harness-resumed"]') .waitFor({ state: "visible", timeout: uiTimeoutMs }); - const continuationRemoved = await page.evaluate((stateKey) => { + const continuationRemoved = await page.evaluate(async (stateKey) => { const plugin = (globalThis as ReviewHarnessTestGlobal).app?.plugins?.plugins["obsidian-livesync"]; if (typeof plugin !== "object" || plugin === null || !("core" in plugin)) { throw new Error("Self-hosted LiveSync is unavailable after restart."); } - const core = (plugin as { core: { services: { setting: { getSmallConfig(key: string): string } } } }).core; - return core.services.setting.getSmallConfig(stateKey) === ""; + const core = ( + plugin as { + core: { + services: { + setting: { config: { getValue(key: string, defaultValue: string): Promise } }; + }; + }; + } + ).core; + return (await core.services.setting.config.getValue(stateKey, "")) === ""; }, REVIEW_HARNESS_STATE_KEY); if (!continuationRemoved) throw new Error("The one-shot continuation was not removed before use."); await assertNoHorizontalOverflow(page, harness, { label: "resumed Review Harness" }); diff --git a/test/e2e-obsidian/scripts/settings-ui.ts b/test/e2e-obsidian/scripts/settings-ui.ts index c4be1651..0b930234 100644 --- a/test/e2e-obsidian/scripts/settings-ui.ts +++ b/test/e2e-obsidian/scripts/settings-ui.ts @@ -58,8 +58,9 @@ type LiveSyncTestPlugin = { usePowerUserMode: boolean; useEdgeCaseMode: boolean; hashCacheMaxCount: number; + customChunkSize: number; }; - getSmallConfig(key: string): string | null; + config: { getValue(key: string, defaultValue: string): Promise }; }; }; }; @@ -484,12 +485,12 @@ async function verifyCompatibilityReview(): Promise { }); await withObsidianPage(port, async (page) => { - const markerBeforeAcknowledgement = await page.evaluate(() => { + const markerBeforeAcknowledgement = await page.evaluate(async () => { const plugin = (globalThis as ObsidianTestGlobal).app?.plugins?.plugins["obsidian-livesync"]; if (plugin === undefined) throw new Error("Self-hosted LiveSync is unavailable"); - return plugin.core.services.setting.getSmallConfig("database-compatibility-version"); + return await plugin.core.services.setting.config.getValue("database-compatibility-version", ""); }); - if (markerBeforeAcknowledgement !== null && markerBeforeAcknowledgement !== "") { + if (markerBeforeAcknowledgement !== "") { throw new Error( `The database version was marked as acknowledged before review: ${markerBeforeAcknowledgement}` ); @@ -579,12 +580,12 @@ async function verifyCompatibilityReview(): Promise { .click(); await summary.waitFor({ state: "hidden", timeout: uiTimeoutMs }); await page.waitForFunction( - (expectedVersion) => { + async (expectedVersion) => { const plugin = (globalThis as ObsidianTestGlobal).app?.plugins?.plugins["obsidian-livesync"]; if (plugin === undefined) return false; const setting = plugin.core.services.setting; return ( - setting.getSmallConfig("database-compatibility-version") === expectedVersion && + (await setting.config.getValue("database-compatibility-version", "")) === expectedVersion && setting.currentSettings().versionUpFlash === "" ); }, @@ -611,10 +612,16 @@ async function verifyConfigDoctorFollowsCompatibilityReview(): Promise { state: "visible", timeout: uiTimeoutMs, }); - await doctor.getByText("Enhance chunk size", { exact: true }).waitFor({ - state: "visible", - timeout: uiTimeoutMs, + const chunkSize = await page.evaluate(() => { + const plugin = (globalThis as ObsidianTestGlobal).app?.plugins?.plugins["obsidian-livesync"]; + return plugin?.core.services.setting.currentSettings().customChunkSize; }); + if (chunkSize !== 0) { + throw new Error("Compatibility review must retain the fixture's zero chunk-size value."); + } + if ((await doctor.getByText("Enhance chunk size", { exact: true }).count()) !== 0) { + throw new Error("Config Doctor must accept the recommended zero chunk-size value."); + } if ((await doctor.getByText("Data Compression", { exact: true }).count()) !== 0) { throw new Error("Config Doctor still treats supported Data Compression as a problem."); } @@ -978,6 +985,7 @@ function createSettingsPluginData(settingsOnlyRun: boolean): Record