test(e2e): isolate and streamline two-client sync scenarios - #146
Merged
ClaudiaFang merged 2 commits intoAug 31, 2026
Merged
Conversation
TwoClient's refresh() bypassed vault-folder filtering entirely
(filterFilesByVaultFolder/filterPathByVaultFolder were no-ops) and the
fixture left rootPath/vaultFolder empty, so every refresh() classified
the WHOLE shared branch's remote tree, not just this run's
e2e-tc-<runId>/ namespace. Other suites' leftover fixtures inflated
tree-listing/refresh time and risked the 120s per-test timeout.
Scope both sides via the real production rootPath/vaultFolder model
instead of a test-only filter: createSyncManagerFixture({ scoped: true })
configures the git service's own rootPath (env.ts contexts now accept
one) and settings.vaultFolder to the same e2e-tc-<runId> value, so the
vaultFolder-strip / rootPath-readd round trip cancels out and push/pull
targets stay unchanged while remote-tree classification is actually
scoped. TwoClient's refresh/filter wiring now mirrors main.ts's real
filterFilesByVaultFolder/filterPathByVaultFolder/getNormalizedPath/
getVaultPath instead of bypassing them.
Also adds a fail-fast scope-leakage assertion after every refresh() and
opt-in timing diagnostics (E2E_TIMING_DEBUG=1) around refresh/sync/
baseline, so a future regression or slow run is attributable instead of
surfacing only as a suite timeout.
E2E fixture/support/diagnostics only — no changes to E2E_TEST_TIMEOUT_MS,
retry policy, or production sync code.
npx eslint . — 0 errors (1 pre-existing unrelated warning)
npm run build — passed
npx vitest run — 68 files / 862 tests passed
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SNTkwXym3gvHDn8rhhSPap
Follow-up to the scope-isolation commit, per review feedback: - Extracted the vaultFolder path-mapping rules (filterPathByVaultFolder/ filterFilesByVaultFolder/getNormalizedVaultPath/ getVaultPathFromNormalized) into a pure module src/logic/sync/vault-folder-scope.ts, shared by src/main.ts, SyncScanner.toRepoPath (both now delegate, behavior unchanged), and the E2E TwoClient wiring (now imports the same functions instead of a hand-copied duplicate) — production and the E2E fixture can no longer silently drift apart on this logic. - P0-1: removed the redundant second baseline (`other` was baselined then immediately treated as "A creates a new file", which was really exercising modify, not create); `other` is now a genuine create, one fewer real provider push + verifier read. - P0-2: removed its trailing expectIdempotent + second expectTwoClientConvergence — idempotency-under-repeated-sync is already covered by P0-1's own expectIdempotent; P0-2's actual contract (concurrent edits on different files both survive) is already proven by the first convergence check + explicit remote-content assertions. - convergence-assertions.ts: added captureRemoteSnapshot/RemoteSnapshot so expectConverged/expectMetadataConsistent share one getFile-per-path + one listFiles instead of each independently re-fetching the same remote files; expectTwoClientConvergence now captures once and passes it to both. - Wrapped captureRemoteSnapshot in the existing opt-in timed() helper as "remote snapshot (verifier)" for the same E2E_TIMING_DEBUG=1 breakdown. Not done this round (per plan): no GitLab server-side rootPath listing optimization, no timeout/retry changes, no production sync semantics changes — the main.ts/SyncScanner.ts edits are a pure logic-preserving extraction only. npx eslint . — 0 errors (1 pre-existing unrelated warning) npm run build — passed npx vitest run — 68 files / 862 tests passed Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SNTkwXym3gvHDn8rhhSPap
ClaudiaFang
changed the title
|
ClaudiaFang
marked this pull request as ready for review
August 31, 2026 09:47
ClaudiaFang
merged commit Aug 31, 2026
5422666
into
claude/source-control-foundation
12 checks passed
Member
Author
|
🎉 This PR is included in version 1.6.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two rounds on this PR, per review feedback:
Round 1 — scope isolation
TwoClient's refresh() bypassed vault-folder filtering entirely and the fixture leftrootPath/vaultFolderempty, so everyrefresh()classified the whole shared branch's remote tree, not just this run'se2e-tc-<runId>/namespace.createSyncManagerFixture({ scoped: true })configures the git service's ownrootPathandsettings.vaultFolderto the samee2e-tc-<runId>value, so the vaultFolder-strip / rootPath-readd round trip cancels out symmetrically (push/pull targets unchanged, remote-tree classification now actually scoped).refresh(), and opt-in timing diagnostics (E2E_TIMING_DEBUG=1).Round 2 — streamline
src/logic/sync/vault-folder-scope.ts, used bysrc/main.ts,SyncScanner.toRepoPath(both now delegate, behavior unchanged), and the E2ETwoClientwiring — production and the E2E fixture can no longer silently drift apart on this logic.expectIdempotent+ second convergence check — already fully covered by P0-1's own idempotency assertion; P0-2's real contract (concurrent edits on different files both survive) was already proven without it.convergence-assertions.ts: addedcaptureRemoteSnapshotsoexpectConverged/expectMetadataConsistentshare onegetFile-per-path + onelistFilesinstead of each independently re-fetching the same remote files.Explicitly not done: no
E2E_TEST_TIMEOUT_MS/retry changes, no GitLab-provider-side server-siderootPathlisting optimization, no production sync semantics changes (themain.ts/SyncScanner.tsedits are a pure logic-preserving extraction).Test plan
npx eslint .— 0 errors (1 pre-existing unrelated warning)npm run build(tsc + Obsidian 1.11.0 compat typecheck + esbuild) — passednpx vitest run— 68 files / 862 tests passedscripts/run-e2e.sh --provider gitlab(and github/gitea) against a real provisioned branch/CI — not run in this environment (no Docker daemon / provider credentials here); needed to confirm P0-1..P0-5 land in the target ranges and GitLab stops hitting 120s in practice.🤖 Generated with Claude Code
https://claude.ai/code/session_01SNTkwXym3gvHDn8rhhSPap