From 8e506fdbc70beceb2b05231f9991803116869f35 Mon Sep 17 00:00:00 2001 From: vorotamoroz Date: Tue, 1 Sep 2026 12:08:50 +0000 Subject: [PATCH] Retire redundant P2P Review Harness check --- .../ReviewHarness/reviewHarnessContract.ts | 8 ---- .../reviewHarnessContract.unit.spec.ts | 1 - .../ReviewHarness/reviewHarnessController.ts | 42 +------------------ .../reviewHarnessController.unit.spec.ts | 6 +-- src/main.ts | 2 +- src/serviceFeatures/useReviewHarness.ts | 7 ---- .../useReviewHarness.unit.spec.ts | 3 +- test/e2e-obsidian/README.md | 2 +- test/e2e-obsidian/scripts/review-harness.ts | 2 +- 9 files changed, 6 insertions(+), 67 deletions(-) diff --git a/src/features/ReviewHarness/reviewHarnessContract.ts b/src/features/ReviewHarness/reviewHarnessContract.ts index c9faa1e7..29a76a8c 100644 --- a/src/features/ReviewHarness/reviewHarnessContract.ts +++ b/src/features/ReviewHarness/reviewHarnessContract.ts @@ -24,14 +24,6 @@ export const REVIEW_HARNESS_SCENARIOS = [ mode: "guided", access: "device-local-state", }, - { - id: "p2p-composition", - title: "P2P composition", - description: - "Checks that the Obsidian host and P2P interface still resolve the current Commonlib replicator.", - mode: "automatic", - access: "read-only", - }, { id: "vault-round-trip", title: "Vault fixture round trip", diff --git a/src/features/ReviewHarness/reviewHarnessContract.unit.spec.ts b/src/features/ReviewHarness/reviewHarnessContract.unit.spec.ts index f10ebe38..a24b53e1 100644 --- a/src/features/ReviewHarness/reviewHarnessContract.unit.spec.ts +++ b/src/features/ReviewHarness/reviewHarnessContract.unit.spec.ts @@ -74,7 +74,6 @@ describe("Review Harness contract", () => { expect(REVIEW_HARNESS_SCENARIO_IDS).toEqual([ "settings-lifecycle", "compatibility-review", - "p2p-composition", "vault-round-trip", ]); }); diff --git a/src/features/ReviewHarness/reviewHarnessController.ts b/src/features/ReviewHarness/reviewHarnessController.ts index 6b84e0c1..7bfc755c 100644 --- a/src/features/ReviewHarness/reviewHarnessController.ts +++ b/src/features/ReviewHarness/reviewHarnessController.ts @@ -20,11 +20,6 @@ export interface ReviewHarnessRuntime { isCompatibilityReviewInitialised(): boolean; getCompatibilityPause(): CompatibilityPause | undefined; openCompatibilityReview(): Promise; - getP2PComposition(): { - readonly first: unknown; - readonly second: unknown; - readonly expectedServices: unknown; - }; runVaultRoundTrip(): Promise; readContinuation(): string | null; writeContinuation(value: string): void; @@ -63,37 +58,6 @@ function initialResults(): Record; } -function inspectP2PComposition(input: ReturnType): ReviewHarnessScenarioResult { - if (input.first !== input.second) { - return { - status: "failed", - detail: "Two consecutive reads resolved different P2P replicators without a lifecycle transition.", - observations: [], - }; - } - if (typeof input.first !== "object" || input.first === null) { - return { - status: "failed", - detail: "The P2P composition did not expose a current replicator.", - observations: [], - }; - } - const env = "env" in input.first ? input.first.env : undefined; - const services = typeof env === "object" && env !== null && "services" in env ? env.services : undefined; - if (services !== input.expectedServices) { - return { - status: "failed", - detail: "The current P2P replicator is not bound to the active Obsidian services.", - observations: [], - }; - } - return { - status: "passed", - detail: "The live P2P result resolves the current replicator and active Obsidian services.", - observations: [], - }; -} - export class ReviewHarnessController { private readonly results = initialResults(); private readonly transcript: ReviewHarnessTranscriptEntry[] = []; @@ -163,9 +127,7 @@ export class ReviewHarnessController { } async runAutomaticScenarios(): Promise { - for (const id of ["settings-lifecycle", "p2p-composition"] as const) { - await this.runScenario(id); - } + await this.runScenario("settings-lifecycle"); } async runAllScenarios(): Promise { @@ -195,8 +157,6 @@ export class ReviewHarnessController { settings: this.runtime.getSettings(), newVaultSettings: this.runtime.getNewVaultSettings(), }); - } else if (id === "p2p-composition") { - result = inspectP2PComposition(this.runtime.getP2PComposition()); } else if (id === "vault-round-trip") { result = await this.runtime.runVaultRoundTrip(); } else { diff --git a/src/features/ReviewHarness/reviewHarnessController.unit.spec.ts b/src/features/ReviewHarness/reviewHarnessController.unit.spec.ts index f80fa183..04b07bee 100644 --- a/src/features/ReviewHarness/reviewHarnessController.unit.spec.ts +++ b/src/features/ReviewHarness/reviewHarnessController.unit.spec.ts @@ -43,8 +43,6 @@ function createRuntime(): ReviewHarnessRuntime & { continuation: string | null; events: string[]; } { - const services = {}; - const replicator = { env: { services } }; const runtime: ReviewHarnessRuntime & { compatibilityReviewInitialised: boolean; compatibilityPause: CompatibilityPause | undefined; @@ -77,7 +75,6 @@ function createRuntime(): ReviewHarnessRuntime & { runtime.events.push("open-compatibility-review"); runtime.compatibilityPause = undefined; }), - getP2PComposition: () => ({ first: replicator, second: replicator, expectedServices: services }), runVaultRoundTrip: vi.fn(async () => ({ status: "passed" as const, detail: "The owned fixture tree was exercised and removed.", @@ -110,14 +107,13 @@ function createRuntime(): ReviewHarnessRuntime & { } describe("ReviewHarnessController", () => { - it("runs the automatic settings and P2P composition checks", async () => { + it("runs the automatic settings check", async () => { const runtime = createRuntime(); const controller = new ReviewHarnessController(runtime); await controller.runAutomaticScenarios(); expect(controller.snapshot().results["settings-lifecycle"].status).toBe("passed"); - expect(controller.snapshot().results["p2p-composition"].status).toBe("passed"); expect(controller.snapshot().results["vault-round-trip"].status).toBe("idle"); expect(controller.snapshot().results["compatibility-review"].status).toBe("idle"); }); diff --git a/src/main.ts b/src/main.ts index 26f2a957..a3216b86 100644 --- a/src/main.ts +++ b/src/main.ts @@ -201,7 +201,7 @@ export default class ObsidianLiveSyncPlugin extends Plugin { createObsidianCompatibilityReviewUi(core.confirm) ); waitForCompatibilityReview = () => compatibilityReview.openReview(); - useReviewHarness(core, this, replicator, compatibilityReview); + useReviewHarness(core, this, compatibilityReview); } ); } diff --git a/src/serviceFeatures/useReviewHarness.ts b/src/serviceFeatures/useReviewHarness.ts index 04d42755..2f9c9791 100644 --- a/src/serviceFeatures/useReviewHarness.ts +++ b/src/serviceFeatures/useReviewHarness.ts @@ -1,6 +1,5 @@ import { NEW_VAULT_SETTINGS } from "@vrtmrz/livesync-commonlib/settings"; import { LOG_LEVEL_NOTICE } from "octagonal-wheels/common/logger"; -import type { UseP2PReplicatorResult } from "@vrtmrz/livesync-commonlib/compat/replication/trystero/UseP2PReplicatorResult"; import type ObsidianLiveSyncPlugin from "@/main"; import type { LiveSyncCore } from "@/main"; import type { WorkspaceLeaf } from "@/deps"; @@ -47,7 +46,6 @@ async function runVaultRoundTrip(plugin: ObsidianLiveSyncPlugin): Promise compatibilityReview.initialised, getCompatibilityPause: () => compatibilityReview.pendingPause, openCompatibilityReview: () => compatibilityReview.openReview(), - getP2PComposition: () => ({ - first: p2p.replicator, - second: p2p.replicator, - expectedServices: services, - }), runVaultRoundTrip: () => runVaultRoundTrip(plugin), readContinuation: () => services.setting.getSmallConfig(REVIEW_HARNESS_STATE_KEY), writeContinuation: (value) => services.setting.setSmallConfig(REVIEW_HARNESS_STATE_KEY, value), diff --git a/src/serviceFeatures/useReviewHarness.unit.spec.ts b/src/serviceFeatures/useReviewHarness.unit.spec.ts index 1b3f48e4..35281081 100644 --- a/src/serviceFeatures/useReviewHarness.unit.spec.ts +++ b/src/serviceFeatures/useReviewHarness.unit.spec.ts @@ -42,7 +42,6 @@ function createFixture(options: { enableDebugTools?: boolean; continuation?: str periodicReplication: true, }; const services = {} as Record; - const replicator = { env: { services } }; const api = { registerWindow: vi.fn(), addCommand: vi.fn(), @@ -101,7 +100,7 @@ function createFixture(options: { enableDebugTools?: boolean; continuation?: str }, }; - const controller = useReviewHarness(core as never, plugin as never, { replicator } as never, compatibilityReview as never); + const controller = useReviewHarness(core as never, plugin as never, compatibilityReview as never); return { controller, api, diff --git a/test/e2e-obsidian/README.md b/test/e2e-obsidian/README.md index 1b8f74f7..8139c056 100644 --- a/test/e2e-obsidian/README.md +++ b/test/e2e-obsidian/README.md @@ -121,7 +121,7 @@ The native run writes `settings-declarative-landing.png`, `settings-declarative- The mobile pass uses Obsidian's `app.emulateMobile(true)`, a 390 by 844 CSS-pixel viewport, and explicit iPhone-style safe-area insets of 47 pixels at the top and 34 pixels at the bottom. The public `@vrtmrz/obsidian-test-session` layout assertions require each modal to remain within the viewport and safe area without horizontal overflow. They also require the Obsidian Close control to remain within the safe area and provide at least a 44 by 44 CSS-pixel touch target. The runner clicks that control to verify actionability, then completes the explicit cancellation path. These simulated checks cover deterministic layout and interaction boundaries; they do not claim to reproduce a native operating-system overlay. -`test:e2e:obsidian:review-harness` exercises only the boundaries owned by the opt-in maintainer Harness. It retains a real compatibility pause, uses the fixed Harness restart action to persist a device-local continuation and reload Obsidian, and requires the Harness to delete that state before reopening. It also runs the bounded local observations, confirms the dedicated Vault fixture root is removed, captures the copied privacy-bounded Markdown report, and checks the Harness layout and touch targets in mobile test mode. Compatibility explanation and persistence details remain owned by `settings-ui`, real P2P transfer remains owned by the dedicated P2P suites, and general Vault reflection remains owned by `vault-reflection`; the Harness test does not duplicate those workflows. +`test:e2e:obsidian:review-harness` exercises only the boundaries owned by the opt-in maintainer Harness. It retains a real compatibility pause, uses the fixed Harness restart action to persist a device-local continuation and reload Obsidian, and requires the Harness to delete that state before reopening. It also runs the bounded settings-lifecycle observation, confirms the dedicated Vault fixture root is removed, captures the copied privacy-bounded Markdown report, and checks the Harness layout and touch targets in mobile test mode. Compatibility explanation and persistence details remain owned by `settings-ui`, real P2P transfer remains owned by the dedicated P2P suites, and general Vault reflection remains owned by `vault-reflection`; the Harness test does not duplicate those workflows. `test:e2e:obsidian:p2p-pane` starts one configured CouchDB-only session with no P2P profile and separate configured P2P sessions for desktop and mobile. It proves that the command remains registered while the retired command, automatic pane, and ribbon entry without a P2P configuration are absent. For the configured P2P profiles, it verifies that the desktop ribbon is available, the current status command reaches the pane without it opening at start-up, checks its connection control and horizontal layout, and captures unobstructed desktop and mobile screenshots. The mobile session uses a fresh Vault, profile, and Obsidian process, enters `app.emulateMobile(true)` through `lifecycle.beforePluginStart`, and requires the P2P view to belong to the right drawer rather than inheriting desktop workspace state. It deliberately uses no relay or peer: replacement of the active replicator is covered by focused unit tests, the Deno and Compose CLI P2P lifecycle suite covers the headless transport, and `p2p-setup-uri-workflow` owns the visible transfer path between two real Obsidian sessions. diff --git a/test/e2e-obsidian/scripts/review-harness.ts b/test/e2e-obsidian/scripts/review-harness.ts index 3c7fb96f..383074c7 100644 --- a/test/e2e-obsidian/scripts/review-harness.ts +++ b/test/e2e-obsidian/scripts/review-harness.ts @@ -196,7 +196,7 @@ async function runAutomaticScenarios(): Promise { await withObsidianPage(obsidianRemoteDebuggingPort(), async (page) => { const harness = page.locator('[data-testid="review-harness"]'); await harness.locator('[data-testid="review-harness-run-automatic"]').click({ timeout: uiTimeoutMs }); - for (const id of ["settings-lifecycle", "p2p-composition"]) { + for (const id of ["settings-lifecycle"]) { await harness .locator(`[data-testid="review-harness-result-${id}"]`) .getByText("Passed:", { exact: false })