diff --git a/e2e-tests/provider/config/env.ts b/e2e-tests/provider/config/env.ts index 3566c62..ab938ab 100644 --- a/e2e-tests/provider/config/env.ts +++ b/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); } diff --git a/e2e-tests/provider/suites/two-client-sync.e2e.test.ts b/e2e-tests/provider/suites/two-client-sync.e2e.test.ts index f4225c9..4d1266b 100644 --- a/e2e-tests/provider/suites/two-client-sync.e2e.test.ts +++ b/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 ---------------------------- diff --git a/e2e-tests/provider/support/convergence-assertions.ts b/e2e-tests/provider/support/convergence-assertions.ts index ab719ea..d9ee134 100644 --- a/e2e-tests/provider/support/convergence-assertions.ts +++ b/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,35 +36,64 @@ export async function trackedPaths(context: ConvergenceContext): Promise 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; + /** All remote paths under this run's namespace, one `listFiles` call. */ + remotePaths: string[]; +} + +export async function captureRemoteSnapshot(context: ConvergenceContext, paths?: string[]): Promise { + return timed('remote snapshot (verifier)', async () => { + const trackedPathList = paths ?? (await trackedPaths(context)); + const files = new Map(); + 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 { +export async function expectConverged(context: ConvergenceContext, snapshot?: RemoteSnapshot): Promise { 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))); } /** @@ -71,17 +101,18 @@ export async function expectConverged(context: ConvergenceContext): Promise { +export async function expectMetadataConsistent(context: ConvergenceContext, snapshot?: RemoteSnapshot): Promise { 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 { - 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); } diff --git a/e2e-tests/provider/support/sync-manager-fixture.ts b/e2e-tests/provider/support/sync-manager-fixture.ts index ed5e738..3a416f4 100644 --- a/e2e-tests/provider/support/sync-manager-fixture.ts +++ b/e2e-tests/provider/support/sync-manager-fixture.ts @@ -56,9 +56,29 @@ export interface SyncManagerFixture { conflictResolver(): (conflict: BatchPushConflict) => ConflictResolution; } -export async function createSyncManagerFixture(): Promise { +export interface SyncManagerFixtureOptions { + /** + * Scopes both the service's remote-tree listing (`rootPath`) and local + * vault discovery (`vaultFolder`) to this fixture's own `e2e-tc-` + * 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 { 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 { 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 { giteaToken: '', giteaBaseUrl: '', giteaOwner: '', giteaRepo: '', branch: branchOverride ?? branch, syncMetadata: {}, - rootPath: '', - vaultFolder: '', + rootPath: scopePath, + vaultFolder: scopePath, symlinkHandling: 'skip', ignorePatterns: '', lastSeenVersion: '', diff --git a/e2e-tests/provider/support/timing-diagnostics.ts b/e2e-tests/provider/support/timing-diagnostics.ts new file mode 100644 index 0000000..b92ac9d --- /dev/null +++ b/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(label: string, fn: () => Promise): Promise { + if (!enabled) return fn(); + const start = Date.now(); + try { + return await fn(); + } finally { + console.log(`[e2e-timing] ${label}: ${Date.now() - start}ms`); + } +} diff --git a/e2e-tests/provider/support/two-client-sync-scenario.ts b/e2e-tests/provider/support/two-client-sync-scenario.ts index d150916..d74a545 100644 --- a/e2e-tests/provider/support/two-client-sync-scenario.ts +++ b/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-` 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 { - 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-` 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 { 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 { 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`); diff --git a/progress.md b/progress.md index 12f505f..8c9aff4 100644 --- a/progress.md +++ b/progress.md @@ -21,6 +21,32 @@ Below that: the previous "Outstanding Items"/"Verification Evidence" entries tra ## Verification Evidence +This session (follow-up round on the same PR — `test(e2e): isolate and streamline two-client sync scenarios`): + +- Phase 1 (correctness, requested follow-up to the prior round): extracted the vaultFolder path-mapping rules (`filterPathByVaultFolder`/`filterFilesByVaultFolder`/`getNormalizedVaultPath`/`getVaultPathFromNormalized`) into a new pure module `src/logic/sync/vault-folder-scope.ts`, shared by `src/main.ts` (delegates now, behavior unchanged), `SyncScanner.toRepoPath` (delegates now, behavior unchanged), and `two-client-sync-scenario.ts`'s `TwoClient` wiring (now imports the same functions instead of a hand-copied duplicate) — so production and the E2E fixture can never silently drift apart on this logic again. +- Phase 2 (remove redundant work): + - P0-1: dropped the second `s.baseline(other, ...)` — `other` was baselined then immediately treated as "A creates a new file", which was actually exercising modify, not create. `other` is now a genuine create (never baselined), one fewer real provider push + verifier read, and the test now actually covers the create→remote→pull path its comment claims. + - P0-2: dropped its trailing `expectIdempotent(ctx)` + second `expectTwoClientConvergence(ctx)` — idempotency-under-repeated-sync is already covered by P0-1's own `expectIdempotent`; P0-2's contract is "concurrent edits on different files both survive", which the first `expectTwoClientConvergence` + explicit remote-content checks already prove. Removes 3 extra full sync rounds (`A.sync/B.sync/A.sync`) worth of provider round trips per run. + - `convergence-assertions.ts`: added `captureRemoteSnapshot`/`RemoteSnapshot` — one `getFile` per tracked path + one `listFiles`, captured once — and `expectConverged`/`expectMetadataConsistent` now accept an optional snapshot instead of each independently re-fetching the same remote files. `expectTwoClientConvergence` captures one snapshot and passes it to both, roughly halving the verifier calls per convergence check. +- Phase 3 (measurement): `captureRemoteSnapshot` is now wrapped in the existing opt-in `timed()` helper (`E2E_TIMING_DEBUG=1`) as `"remote snapshot (verifier)"`, alongside the prior round's `refresh`/`sync`/`baseline` timings — covers the plan's tree-listing/refresh/push/pull/verifier attribution list. Per-test total duration is already reported natively by vitest's own output; not hand-rolled separately. +- Explicitly NOT done this round (per plan): no GitLab-provider-side server-side `rootPath` tree-listing optimization, no timeout/retry changes, no production sync **semantics** changes — `main.ts`/`SyncScanner.ts` changes here are a pure logic-preserving extraction only. +- `npx eslint .` — 0 errors, 1 pre-existing unrelated warning (`obsidian-request-url.ts`'s unused `_T` generic). +- `npm run build` (tsc + Obsidian 1.11.0 compat typecheck + esbuild) — passed. +- `npx vitest run` — 68 files / 862 tests passed (same count as before this round — the extraction is behavior-preserving, no new/removed unit tests). +- **Not verified in this environment**: a real multi-suite E2E run proving the new P0-1/P0-2 timings land in the plan's target ranges (35–50s / 25–40s) and that GitLab stops hitting 120s — needs `scripts/run-e2e.sh --provider gitlab|github|gitea` against a real provisioned branch/CI (no Docker daemon / provider credentials in this environment). + +This session (test(e2e): isolate two-client sync scope — follow-up to the CI-run-33358507732 triage): + +- Root cause of the P0-1..P0-5 slowness/timeout risk: `two-client-sync.e2e.test.ts`'s fixture (`createSyncManagerFixture()`) built settings with `rootPath: ''`/`vaultFolder: ''`, and `TwoClient`'s `SyncStatusRefreshService` wiring bypassed vault-folder filtering entirely (`filterFilesByVaultFolder: files => files`, `filterPathByVaultFolder: () => true`). Every `refresh()` therefore listed and classified the WHOLE shared branch's remote tree — every other suite's `e2e-sc-*` fixtures included — not just this run's `e2e-tc-/` namespace. +- Fixed via the real production rootPath/vaultFolder model, not a test-only filter: `createSyncManagerFixture({ scoped: true })` (new opt-in option, `e2e-tests/provider/support/sync-manager-fixture.ts`) now generates its `runId` up front and configures BOTH the git service's own `rootPath` (`e2e-tests/provider/config/env.ts`'s `contextFor`/`githubContext`/`gitlabContext`/`giteaContext` now take a `rootPath` param, threaded into `service.updateConfig`) and `settings.vaultFolder` to the same `e2e-tc-` value. Because `vaultFolder` and `rootPath` are set identically, the local-vault-path ⇄ repo-relative-path round trip cancels out symmetrically: `SyncScanner.toRepoPath` strips `vaultFolder` before calling the service, and the service's own `rootPath` re-adds the same prefix when resolving the real remote path — so push/pull targets are unchanged, but `SyncStatusRefreshService.getNormalizedRemotePath` (already reading `settings().rootPath`) now actually scopes remote-tree classification, and `filterFilesByVaultFolder`/`filterPathByVaultFolder`/`getNormalizedPath`/`getVaultPath` in `two-client-sync-scenario.ts`'s `TwoClient` wiring were changed from test-only bypasses to the same vaultFolder-prefix logic `src/main.ts` uses in production. +- Added a fail-fast scope-leakage guard: `TwoClient.refresh()` now asserts every classified change's path starts with `e2e-tc-/` immediately after refresh, so a future regression in this isolation fails in seconds instead of surfacing as a 120s suite timeout. +- Added opt-in timing diagnostics (`e2e-tests/provider/support/timing-diagnostics.ts`, gated on `E2E_TIMING_DEBUG=1`, silent otherwise) around `refresh`/`sync`/`baseline`, so a future slow CI run can be attributed to a specific phase (tree listing / refresh / push / pull) instead of only "the test approached 120s". +- Scope: E2E fixture/support/diagnostics only — did not touch `E2E_TEST_TIMEOUT_MS`, retry policy, or `src/` production sync code. `path()`-based test bodies in `two-client-sync.e2e.test.ts` (P0-1..P0-5) needed no changes — the vaultFolder/rootPath symmetry keeps their existing `s.path('...')` full-path convention working unchanged. +- `npx eslint .` — 0 errors, 1 pre-existing unrelated warning (`obsidian-request-url.ts`'s unused `_T` generic). +- `npm run build` (tsc + Obsidian 1.11.0 compat typecheck + esbuild) — passed. +- `npx vitest run` — 68 files / 862 tests passed. +- **Not verified in this environment**: an actual multi-suite-sharing-one-branch E2E run proving the leakage is gone in practice (needs `scripts/run-e2e.sh --provider gitlab|github|gitea` against a real provisioned branch/CI, per the plan's verification matrix — this environment has no Docker daemon / provider credentials). + This session (#142 — e2e/ → e2e-tests/provider/ scanner-boundary move, static runtime files): - Moved `e2e/{config,shim,suites,support}` → `e2e-tests/provider/{config,shim,suites,support}` (git mv, history preserved); deleted `e2e/runtime-modules.d.ts` and `e2e/verifier-runtime-types.ts`. diff --git a/src/logic/sync/SyncScanner.ts b/src/logic/sync/SyncScanner.ts index 60133c7..0ef73fc 100644 --- a/src/logic/sync/SyncScanner.ts +++ b/src/logic/sync/SyncScanner.ts @@ -2,6 +2,7 @@ import { TFile, type App } from 'obsidian'; import type { GitLabFilesPushSettings } from '../../settings'; import { logger } from '../../utils/logger'; import { isBinaryPath } from '../../utils/path'; +import { getNormalizedVaultPath } from './vault-folder-scope'; export interface ScannedFileInfo { path: string; @@ -24,10 +25,7 @@ export class SyncScanner { } toRepoPath(path: string): string { - if (!this.settings.vaultFolder) return path; - const folderPath = `${this.settings.vaultFolder}/`; - if (path.startsWith(folderPath)) return path.substring(folderPath.length); - return path === this.settings.vaultFolder ? '' : path; + return getNormalizedVaultPath(path, this.settings.vaultFolder); } toTreePath(repoPath: string): string { diff --git a/src/logic/sync/vault-folder-scope.ts b/src/logic/sync/vault-folder-scope.ts new file mode 100644 index 0000000..4c38a70 --- /dev/null +++ b/src/logic/sync/vault-folder-scope.ts @@ -0,0 +1,34 @@ +/** + * Pure `vaultFolder` path-mapping rules, shared by the plugin runtime + * (`src/main.ts`) and the real-provider E2E fixtures + * (`e2e-tests/provider/support/two-client-sync-scenario.ts`) so the two never + * drift apart. `vaultFolder` scopes the Obsidian vault to a subfolder of the + * local filesystem/vault-relative namespace; an empty `vaultFolder` means the + * vault root already is the sync root, so every function here is a no-op. + */ + +export function filterPathByVaultFolder(path: string, vaultFolder: string): boolean { + if (!vaultFolder) return true; + const folderPath = `${vaultFolder}/`; + return path.startsWith(folderPath) || path === vaultFolder; +} + +export function filterFilesByVaultFolder(files: T[], vaultFolder: string): T[] { + if (!vaultFolder) return files; + return files.filter(file => filterPathByVaultFolder(file.path, vaultFolder)); +} + +/** Vault-relative path -> repo-relative path (strips the `vaultFolder` prefix). */ +export function getNormalizedVaultPath(path: string, vaultFolder: string): string { + if (!vaultFolder) return path; + const folderPath = `${vaultFolder}/`; + if (path.startsWith(folderPath)) return path.substring(folderPath.length); + return path === vaultFolder ? '' : path; +} + +/** Repo-relative path -> vault-relative path (re-adds the `vaultFolder` prefix). */ +export function getVaultPathFromNormalized(normalizedPath: string, vaultFolder: string): string { + if (!vaultFolder) return normalizedPath; + if (!normalizedPath) return vaultFolder; + return `${vaultFolder}/${normalizedPath}`; +} diff --git a/src/main.ts b/src/main.ts index 8cbcefb..1ac9f5e 100644 --- a/src/main.ts +++ b/src/main.ts @@ -27,6 +27,12 @@ import { SourceControlViewModel } from './logic/source-control/SourceControlView import { SourceControlActionService } from './logic/source-control/SourceControlActionService'; import { SyncResultNotifier } from './logic/source-control/SyncResultNotifier'; import { toSyncChanges } from './logic/source-control/FileStatusAdapter'; +import { + filterFilesByVaultFolder as scopeFilterFiles, + filterPathByVaultFolder as scopeFilterPath, + getNormalizedVaultPath, + getVaultPathFromNormalized, +} from './logic/sync/vault-folder-scope'; export type ConnectionStatusState = 'checking' | 'connected' | 'disconnected'; @@ -626,34 +632,19 @@ export default class GitLabFilesPush extends Plugin { } filterFilesByVaultFolder(files: TFile[]): TFile[] { - if (!this.settings.vaultFolder) { - return files; - } - - const folderPath = this.settings.vaultFolder + '/'; - return files.filter(file => file.path.startsWith(folderPath) || file.path === this.settings.vaultFolder); + return scopeFilterFiles(files, this.settings.vaultFolder); } filterPathByVaultFolder(path: string): boolean { - if (!this.settings.vaultFolder) return true; - const folderPath = this.settings.vaultFolder + '/'; - return path.startsWith(folderPath) || path === this.settings.vaultFolder; + return scopeFilterPath(path, this.settings.vaultFolder); } getNormalizedPath(path: string): string { - if (!this.settings.vaultFolder) return path; - const folderPath = this.settings.vaultFolder + '/'; - if (path.startsWith(folderPath)) { - return path.substring(folderPath.length); - } - if (path === this.settings.vaultFolder) return ''; - return path; + return getNormalizedVaultPath(path, this.settings.vaultFolder); } getVaultPath(normalizedPath: string): string { - if (!this.settings.vaultFolder) return normalizedPath; - if (!normalizedPath) return this.settings.vaultFolder; - return this.settings.vaultFolder + '/' + normalizedPath; + return getVaultPathFromNormalized(normalizedPath, this.settings.vaultFolder); } initializeGitService(): void {