test(e2e): isolate and streamline two-client sync scenarios

e2e-tests/provider/config/env.ts

@@ -60,21 +60,21 @@ export interface ProviderContext {
     branch: string;
 }
 
-export function githubContext(): ProviderContext {
+export function githubContext(rootPath = ''): ProviderContext {
     const owner = requiredEnv('E2E_GITHUB_OWNER');
     const repo = requiredEnv('E2E_GITHUB_REPO');
     const token = requiredEnv('E2E_GITHUB_TOKEN');
     const service = new GitHubService();
-    service.updateConfig(token, owner, repo, '');
+    service.updateConfig(token, owner, repo, rootPath);
     return { service, branch: testBranch() };
 }
 
-export function gitlabContext(): ProviderContext {
+export function gitlabContext(rootPath = ''): ProviderContext {
     const baseUrl = process.env.E2E_GITLAB_BASE_URL ?? 'https://gitlab.com';
     const projectId = requiredEnv('E2E_GITLAB_PROJECT_ID');
     const token = requiredEnv('E2E_GITLAB_TOKEN');
     const service = new GitLabService();
-    service.updateConfig(baseUrl, token, projectId, '');
+    service.updateConfig(baseUrl, token, projectId, rootPath);
     return { service, branch: testBranch() };
 }
 
@@ -84,7 +84,7 @@ export function gitlabContext(): ProviderContext {
  * URL/credentials generically (E2E_TEST_REPO_URL/E2E_GIT_USERNAME/
  * E2E_GIT_TOKEN), since there's no stable owner/repo pair to name ahead of time.
  */
-export function giteaContext(): ProviderContext {
+export function giteaContext(rootPath = ''): ProviderContext {
     const repoUrl = new URL(requiredEnv('E2E_TEST_REPO_URL'));
     const token = requiredEnv('E2E_GIT_TOKEN');
     const [owner, repoWithGit] = repoUrl.pathname.replace(/^\//, '').split('/');
@@ -94,12 +94,19 @@ export function giteaContext(): ProviderContext {
     }
     const baseUrl = `${repoUrl.protocol}//${repoUrl.host}`;
     const service = new GiteaService();
-    service.updateConfig(baseUrl, token, owner, repo, '');
+    service.updateConfig(baseUrl, token, owner, repo, rootPath);
     return { service, branch: testBranch() };
 }
 
-export function contextFor(provider: E2EProvider): ProviderContext {
-    if (provider === 'github') return githubContext();
-    if (provider === 'gitlab') return gitlabContext();
-    return giteaContext();
+/**
+ * `rootPath` scopes the service's own remote-tree listing to a repo
+ * subfolder — the real production mechanism, not a test-only filter. Suites
+ * that share one branch across several fixtures (e.g. multi-client E2E) pass
+ * their run's namespace here so each fixture's service only ever sees its own
+ * files, instead of every suite's files sharing one unscoped listing.
+ */
+export function contextFor(provider: E2EProvider, rootPath = ''): ProviderContext {
+    if (provider === 'github') return githubContext(rootPath);
+    if (provider === 'gitlab') return gitlabContext(rootPath);
+    return giteaContext(rootPath);
 }

e2e-tests/provider/suites/two-client-sync.e2e.test.ts

@@ -38,7 +38,7 @@ describe('Two-client sync E2E', () => {
     let setResolver: (resolution: ConflictResolution) => void;
 
     beforeAll(async () => {
-        fixture = await createSyncManagerFixture();
+        fixture = await createSyncManagerFixture({ scoped: true });
         setResolver = (resolution: ConflictResolution): void => {
             fixture.setConflictResolver(() => resolution);
         };
@@ -66,7 +66,6 @@ describe('Two-client sync E2E', () => {
         const ctx: ConvergenceContext = convergenceContext([s.a, s.b], fixture.verifier, fixture.branch, `e2e-tc-${fixture.runId}/p0-1/`);
 
         await s.baseline(file, 'v1');
-        await s.baseline(other, 'other-v1');
 
         // A edits and syncs; B then pulls.
         s.a.write(file, 'A edit v2');
@@ -109,11 +108,12 @@ describe('Two-client sync E2E', () => {
         await s.b.sync();
         await s.a.sync();
 
+        // Idempotency under repeated syncs of converged state is already
+        // covered by P0-1's expectIdempotent — P0-2's own contract is that
+        // concurrent edits on different files both survive the merge.
         await expectTwoClientConvergence(ctx);
         await s.expectRemoteContent(fileA, 'a-v2 by A');
         await s.expectRemoteContent(fileB, 'b-v2 by B');
-        await expectIdempotent(ctx);
-        await expectTwoClientConvergence(ctx);
     });
 
     // --- P0-3: same-file modify/modify conflict ----------------------------

e2e-tests/provider/support/convergence-assertions.ts

@@ -1,6 +1,7 @@
 import { expect } from 'vitest';
 import type { GitVerifier } from './git-verifier';
 import type { TwoClient } from './two-client-sync-scenario';
+import { timed } from './timing-diagnostics';
 
 /**
  * Multi-client safety invariants, expressed once so every two-client test
@@ -35,53 +36,83 @@ export async function trackedPaths(context: ConvergenceContext): Promise<string[
     return [...paths].sort((a, b) => a.localeCompare(b));
 }
 
+/** A file's remote content/sha, or `null` if it doesn't exist remotely. */
+export type RemoteFile = { content: string; sha: string } | null;
+
+/**
+ * One read of "everything a convergence check needs from the remote", so
+ * `expectConverged` + `expectMetadataConsistent` don't each independently
+ * re-fetch the same files — every extra round trip is real wall-clock time
+ * against the real provider API.
+ */
+export interface RemoteSnapshot {
+    /** Tracked path -> remote file (or null if absent), one fetch per path. */
+    files: Map<string, RemoteFile>;
+    /** All remote paths under this run's namespace, one `listFiles` call. */
+    remotePaths: string[];
+}
+
+export async function captureRemoteSnapshot(context: ConvergenceContext, paths?: string[]): Promise<RemoteSnapshot> {
+    return timed('remote snapshot (verifier)', async () => {
+        const trackedPathList = paths ?? (await trackedPaths(context));
+        const files = new Map<string, RemoteFile>();
+        for (const path of trackedPathList) {
+            files.set(path, await context.verifier.getFile(path, context.branch));
+        }
+        const remotePaths = (await context.verifier.listFiles(context.branch))
+            .filter(path => path.startsWith(context.runPrefix))
+            .sort((a, b) => a.localeCompare(b));
+        return { files, remotePaths };
+    });
+}
+
 /**
  * Invariant A — Convergence: after a complete sync cycle,
  * A local tree == B local tree == remote tree for every tracked path
- * (existence, content, and absence all agree).
+ * (existence, content, and absence all agree). Reuses `snapshot` if given
+ * (see `captureRemoteSnapshot`) instead of re-fetching from the remote.
  */
-export async function expectConverged(context: ConvergenceContext): Promise<void> {
+export async function expectConverged(context: ConvergenceContext, snapshot?: RemoteSnapshot): Promise<void> {
     const [clientA, clientB] = context.clients;
     const paths = await trackedPaths(context);
+    const remote = snapshot ?? await captureRemoteSnapshot(context, paths);
     for (const path of paths) {
-        const remote = await context.verifier.getFile(path, context.branch);
+        const remoteFile = remote.files.get(path) ?? null;
         const aHas = clientA.exists(path);
         const bHas = clientB.exists(path);
         expect(aHas, `convergence: ${path} existence A vs B (${aHas} vs ${bHas})`).toBe(bHas);
         const expectedMessage = `convergence: ${path} local vs remote`;
         if (!aHas) {
-            expect(remote, expectedMessage).toBeNull();
+            expect(remoteFile, expectedMessage).toBeNull();
             continue;
         }
-        expect(remote, expectedMessage).not.toBeNull();
+        expect(remoteFile, expectedMessage).not.toBeNull();
         expect(await clientA.read(path), `convergence: ${path} A vs B`).toBe(await clientB.read(path));
-        expect(await clientA.read(path), `convergence: ${path} A vs remote`).toBe(remote!.content);
+        expect(await clientA.read(path), `convergence: ${path} A vs remote`).toBe(remoteFile!.content);
     }
     // Nothing in the run's remote namespace should exist without existing in
     // both local vaults either (catches remote-only surprises like a dropped
     // rename source that left a stale blob behind).
-    const remoteFiles = (await context.verifier.listFiles(context.branch))
-        .filter(path => path.startsWith(context.runPrefix))
-        .sort((a, b) => a.localeCompare(b));
-    expect(remoteFiles).toEqual(paths.filter(path => clientA.exists(path)));
+    expect(remote.remotePaths).toEqual(paths.filter(path => clientA.exists(path)));
 }
 
 /**
  * Invariant B — Metadata consistency: every path that exists locally in a
  * client (i.e. was synced, not deliberately local-only) must carry a
  * lastSyncedSha equal to the current remote blob sha — on BOTH clients.
  * Catches "file looks identical but baselines diverged" — the source of the
- * next false conflict or silent overwrite.
+ * next false conflict or silent overwrite. Reuses `snapshot` if given.
  */
-export async function expectMetadataConsistent(context: ConvergenceContext): Promise<void> {
+export async function expectMetadataConsistent(context: ConvergenceContext, snapshot?: RemoteSnapshot): Promise<void> {
     const paths = await trackedPaths(context);
+    const remote = snapshot ?? await captureRemoteSnapshot(context, paths);
     for (const path of paths) {
-        const remote = await context.verifier.getFile(path, context.branch);
-        if (!remote) continue;
+        const remoteFile = remote.files.get(path);
+        if (!remoteFile) continue;
         for (const client of context.clients) {
             if (!client.exists(path)) continue;
             const meta = client.metadata(path);
-            expect(meta?.lastSyncedSha, `metadata: ${client.name} ${path} lastSyncedSha vs remote blob sha`).toBe(remote.sha);
+            expect(meta?.lastSyncedSha, `metadata: ${client.name} ${path} lastSyncedSha vs remote blob sha`).toBe(remoteFile.sha);
         }
     }
 }
@@ -140,8 +171,10 @@ export async function expectNoSilentDataLoss(
 
 /** Full post-sync convergence gate used by the P0 suite: A + B + remote together. */
 export async function expectTwoClientConvergence(context: ConvergenceContext): Promise<void> {
-    await expectConverged(context);
-    await expectMetadataConsistent(context);
+    const paths = await trackedPaths(context);
+    const snapshot = await captureRemoteSnapshot(context, paths);
+    await expectConverged(context, snapshot);
+    await expectMetadataConsistent(context, snapshot);
     expectClean(...context.clients);
 }
 

e2e-tests/provider/support/sync-manager-fixture.ts

@@ -56,9 +56,29 @@ export interface SyncManagerFixture {
     conflictResolver(): (conflict: BatchPushConflict) => ConflictResolution;
 }
 
-export async function createSyncManagerFixture(): Promise<SyncManagerFixture> {
+export interface SyncManagerFixtureOptions {
+    /**
+     * Scopes both the service's remote-tree listing (`rootPath`) and local
+     * vault discovery (`vaultFolder`) to this fixture's own `e2e-tc-<runId>`
+     * namespace, via the real production rootPath/vaultFolder model. Needed
+     * by multi-client suites where several independent fixtures/clients share
+     * one branch and must never see each other's remote files — unscoped
+     * (the default) is fine for single-fixture suites, where extra remote
+     * entries from other suites are harmless (they never match a local file).
+     */
+    readonly scoped?: boolean;
+}
+
+export async function createSyncManagerFixture(options: SyncManagerFixtureOptions = {}): Promise<SyncManagerFixture> {
     const provider = currentProvider();
-    const ctx = contextFor(provider);
+
+    // Test-only namespace disambiguator (avoids path collisions between
+    // concurrent e2e runs against the same shared remote) — no security
+    // context, so a non-cryptographic PRNG is intentional here.
+    const runId = Math.random().toString(36).slice(2, 10); // NOSONAR typescript:S2245
+    const scopePath = options.scoped ? `e2e-tc-${runId}` : '';
+
+    const ctx = contextFor(provider, scopePath);
     const service = ctx.service;
     const branch = ctx.branch;
 
@@ -90,13 +110,8 @@ export async function createSyncManagerFixture(): Promise<SyncManagerFixture> {
         return this;
     });
 
-    // Test-only namespace disambiguator (avoids path collisions between
-    // concurrent e2e runs against the same shared remote) — no security
-    // context, so a non-cryptographic PRNG is intentional here.
-    const runId = Math.random().toString(36).slice(2, 10); // NOSONAR typescript:S2245
-
     function path(name: string): string {
-        return `e2e-sc-${runId}/${name}`;
+        return scopePath ? `${scopePath}/${name}` : `e2e-sc-${runId}/${name}`;
     }
 
     function makeSettings(branchOverride?: string): GitLabFilesPushSettings {
@@ -107,8 +122,8 @@ export async function createSyncManagerFixture(): Promise<SyncManagerFixture> {
             giteaToken: '', giteaBaseUrl: '', giteaOwner: '', giteaRepo: '',
             branch: branchOverride ?? branch,
             syncMetadata: {},
-            rootPath: '',
-            vaultFolder: '',
+            rootPath: scopePath,
+            vaultFolder: scopePath,
             symlinkHandling: 'skip',
             ignorePatterns: '',
             lastSeenVersion: '',

e2e-tests/provider/support/timing-diagnostics.ts

@@ -0,0 +1,17 @@
+/**
+ * Opt-in duration logging for the two-client E2E suite. Silent by default —
+ * set `E2E_TIMING_DEBUG=1` to see where a slow run's time actually goes
+ * (tree listing vs refresh vs push vs pull vs verifier), instead of only
+ * knowing a whole test approached the timeout.
+ */
+const enabled = process.env.E2E_TIMING_DEBUG === '1';
+
+export async function timed<T>(label: string, fn: () => Promise<T>): Promise<T> {
+    if (!enabled) return fn();
+    const start = Date.now();
+    try {
+        return await fn();
+    } finally {
+        console.log(`[e2e-timing] ${label}: ${Date.now() - start}ms`);
+    }
+}

e2e-tests/provider/support/two-client-sync-scenario.ts

@@ -16,6 +16,13 @@ import { ChangeRepository } from '../../../src/logic/source-control/ChangeReposi
 import { OperationState } from '../../../src/logic/source-control/OperationState';
 import { SourceControlActionService } from '../../../src/logic/source-control/SourceControlActionService';
 import { toSyncChanges } from '../../../src/logic/source-control/FileStatusAdapter';
+import {
+    filterFilesByVaultFolder,
+    filterPathByVaultFolder,
+    getNormalizedVaultPath,
+    getVaultPathFromNormalized,
+} from '../../../src/logic/sync/vault-folder-scope';
+import { timed } from './timing-diagnostics';
 
 /**
  * The provider-level fixtures the two-client scenario shares across clients.
@@ -83,12 +90,16 @@ export class TwoClient {
                 gitService: () => fixture.service,
                 gitignoreManager: () => gitignoreManager,
                 syncManager: () => this.manager,
-                filterFilesByVaultFolder: files => files,
-                filterPathByVaultFolder: () => true,
-                // vaultFolder/rootPath are empty in e2e settings; vault-relative
-                // path === repo-relative path.
-                getNormalizedPath: path => path,
-                getVaultPath: path => path,
+                // Real production vaultFolder scoping (shared with src/main.ts
+                // and SyncScanner via src/logic/sync/vault-folder-scope) —
+                // this fixture's settings set vaultFolder to this run's own
+                // `e2e-tc-<runId>` namespace, so this scopes local discovery
+                // to this client's own files exactly like a real vault
+                // subfolder mount would.
+                filterFilesByVaultFolder: files => filterFilesByVaultFolder(files, this.settings.vaultFolder),
+                filterPathByVaultFolder: path => filterPathByVaultFolder(path, this.settings.vaultFolder),
+                getNormalizedPath: path => getNormalizedVaultPath(path, this.settings.vaultFolder),
+                getVaultPath: normalizedPath => getVaultPathFromNormalized(normalizedPath, this.settings.vaultFolder),
             },
             this.statuses,
         );
@@ -141,8 +152,25 @@ export class TwoClient {
 
     /** Runs the real Source Control refresh: live local scan + remote tree + per-file classification. */
     async refresh(): Promise<void> {
-        await this.refreshService.refresh();
+        await timed(`refresh ${this.name}`, () => this.refreshService.refresh());
         this.repository.replace(toSyncChanges([...this.statuses.values()]));
+        this.assertScopeIsolation();
+    }
+
+    /**
+     * Fail-fast guard: every change refresh() surfaces must belong to this
+     * run's own `e2e-tc-<runId>` namespace. If fixture/rootPath scoping ever
+     * regresses, this throws immediately instead of the suite timing out
+     * (or, worse, silently asserting on another suite's leaked remote files).
+     */
+    private assertScopeIsolation(): void {
+        const prefix = `e2e-tc-${this.fixture.runId}/`;
+        for (const change of this.repository.getAll()) {
+            expect(
+                change.path.startsWith(prefix),
+                `client ${this.name} refresh() surfaced an out-of-scope change: ${change.path} (expected prefix ${prefix})`,
+            ).toBe(true);
+        }
     }
 
     /** Status rows from the last refresh — the "Repository Changes" view model. */
@@ -162,7 +190,7 @@ export class TwoClient {
     async sync(): Promise<void> {
         await this.refresh();
         const changeIds = this.repository.getAll().map(change => change.id);
-        await this.actionService.sync(changeIds);
+        await timed(`sync ${this.name}`, () => this.actionService.sync(changeIds));
     }
 
     /** Push-only path (the per-row Sync/Push on one or more changes). */
@@ -239,7 +267,7 @@ export class TwoClientSyncScenario {
      */
     async baseline(path: string, content: string): Promise<void> {
         this.a.write(path, content);
-        const result = await this.a.manager.pushFiles([path]);
+        const result = await timed('baseline', () => this.a.manager.pushFiles([path]));
         expect(result.success, `baseline push of ${path} failed: ${JSON.stringify(result.errors)}`).toBe(1);
         const pushedSha = result.syncedPaths.find(entry => entry.path === path)?.sha;
         if (!pushedSha) throw new Error(`baseline push of ${path} did not report a sha`);