From da3c015075f14dec39acfc3e7568a78e7028147a Mon Sep 17 00:00:00 2001 From: tianyao Date: Fri, 7 Aug 2026 10:35:27 +0000 Subject: [PATCH 1/6] test(e2e): add SyncManager real-provider E2E suite Adds e2e/suites/sync-manager.e2e.test.ts, parametrized by E2E_PROVIDER so it reuses the existing github/gitlab/gitea adapters and verifiers rather than duplicating provider-service contract coverage. Real SyncManager + real production provider service; only the Obsidian filesystem boundary is faked (e2e/shim/fake-vault.ts, an in-memory Map, not vi.fn() mocks). Covers: local new file -> push, unchanged file -> no remote mutation, remote update -> pull, conflict protection (push must not silently overwrite, and must not falsely mark synced), rename/move in exactly one commit, delete via the real service (SyncStatusView's actual call path, not a SyncManager method), and batch push-all in exactly one commit. Extends e2e/shim/obsidian-request-url.ts with the minimal set of real `obsidian` values SyncManager's dependency graph needs at runtime (TFile, Notice, Platform, FileSystemAdapter, Modal, plus PluginSettingTab/ TextComponent/AbstractInputSuggest/TFolder/Setting, pulled in transitively via `../settings`'s pure functions sharing a module with the settings-tab UI class) -- see that file's header comment for the full trace. Wires the new suite into scripts/run-e2e.mjs so `npm run test:e2e -- provider ` covers both the contract suite and SyncManager scenarios in one command/container lifecycle. Verified for real: `npm run test:e2e -- --provider gitea` against local Docker, 14/14 passing across multiple consecutive runs. GitHub/GitLab share the same harness but have only been lint/build/typecheck-verified here (no sandbox credentials in this environment) -- see docs/testing/real-provider-e2e.md. --- e2e/shim/fake-vault.ts | 73 ++++++++ e2e/shim/obsidian-request-url.ts | 81 +++++++++ e2e/suites/sync-manager.e2e.test.ts | 256 ++++++++++++++++++++++++++++ scripts/run-e2e.mjs | 11 +- 4 files changed, 420 insertions(+), 1 deletion(-) create mode 100644 e2e/shim/fake-vault.ts create mode 100644 e2e/suites/sync-manager.e2e.test.ts diff --git a/e2e/shim/fake-vault.ts b/e2e/shim/fake-vault.ts new file mode 100644 index 0000000..d103f50 --- /dev/null +++ b/e2e/shim/fake-vault.ts @@ -0,0 +1,73 @@ +import type { App } from 'obsidian'; +import { TFile } from './obsidian-request-url'; + +/** + * Real in-memory Obsidian Vault/App stand-in for SyncManager E2E (see + * e2e/suites/sync-manager.e2e.test.ts) — not a `vi.fn()` mock. The point of + * SyncManager E2E is to exercise real `SyncManager` + real provider service + * code against a real Git server; the *only* thing worth faking is the + * Obsidian filesystem boundary, so this implements exactly the `vault`/ + * `vault.adapter` surface `src/logic/sync-manager.ts` actually touches + * (confirmed by reading it) as a plain `Map`. + */ +export class FakeVault { + private readonly files = new Map(); + + /** Seeds local vault state directly, bypassing any sync logic. */ + writeLocal(path: string, content: string | ArrayBuffer): void { + this.files.set(path, content); + } + + has(path: string): boolean { + return this.files.has(path); + } + + /** Mirrors what Obsidian does on a vault rename: same content, new path. */ + renameLocal(oldPath: string, newPath: string): void { + const content = this.files.get(oldPath); + if (content === undefined) throw new Error(`fake vault: no local file at ${oldPath}`); + this.files.delete(oldPath); + this.files.set(newPath, content); + } + + readonly adapter = { + exists: async (path: string): Promise => this.files.has(path), + read: async (path: string): Promise => { + const content = this.files.get(path); + if (typeof content !== 'string') throw new Error(`fake vault: no text file at ${path}`); + return content; + }, + readBinary: async (path: string): Promise => { + const content = this.files.get(path); + if (!(content instanceof ArrayBuffer)) throw new Error(`fake vault: no binary file at ${path}`); + return content; + }, + write: async (path: string, content: string): Promise => { + this.files.set(path, content); + }, + writeBinary: async (path: string, content: ArrayBuffer): Promise => { + this.files.set(path, content); + }, + // ensureParentDirs (src/utils/vault-path.ts) tolerates mkdir failures; + // there are no real directories to create in an in-memory map. + mkdir: async (): Promise => {}, + }; + + readonly vault = { + read: async (file: TFile): Promise => this.adapter.read(file.path), + readBinary: async (file: TFile): Promise => this.adapter.readBinary(file.path), + modify: async (file: TFile, content: string): Promise => { + this.files.set(file.path, content); + }, + modifyBinary: async (file: TFile, content: ArrayBuffer): Promise => { + this.files.set(file.path, content); + }, + getFileByPath: (path: string): TFile | null => (this.files.has(path) ? new TFile(path) : null), + adapter: this.adapter, + }; +} + +/** Casts a FakeVault into the shape SyncManager expects as its `App` — it only ever touches `app.vault.*`. */ +export function fakeApp(fakeVault: FakeVault): App { + return { vault: fakeVault.vault } as unknown as App; +} diff --git a/e2e/shim/obsidian-request-url.ts b/e2e/shim/obsidian-request-url.ts index 8ac3e0c..1897907 100644 --- a/e2e/shim/obsidian-request-url.ts +++ b/e2e/shim/obsidian-request-url.ts @@ -58,4 +58,85 @@ export async function requestUrl(request: RequestUrlParam | string): Promise `src/settings-implementation.ts` (which bundles those + * pure functions in the same file as the `GitLabSyncSettingTab` UI class) -> + * `src/ui/FolderSuggest.ts`, pulling in `PluginSettingTab`, `TextComponent`, + * `AbstractInputSuggest`, and `TFolder` as real top-level `class X extends Y` + * values too, even though the SyncManager E2E suite never triggers the + * settings UI itself. Splitting those pure functions out of + * settings-implementation.ts to avoid this is a bigger production-code + * change than this E2E harness should make; stubbing the shape here is the + * narrower fix. */ +export class Modal { + app: unknown; + constructor(app?: unknown) { this.app = app; } + open(): void {} + close(): void {} +} + +export class PluginSettingTab { + constructor(_app?: unknown, _plugin?: unknown) {} +} +export class TextComponent {} +export class AbstractInputSuggest<_T> { + constructor(_app: unknown, _inputEl: unknown) {} +} +export class TFolder { + path: string; + constructor(path: string) { this.path = path; } +} +export class Setting { + constructor(_containerEl?: unknown) {} +} + +export class TFile { + path: string; + name: string; + constructor(path: string) { + this.path = path; + this.name = path.split('/').pop() ?? path; + } +} + +export class Notice { + constructor(_message?: string, _timeout?: number) {} + setMessage(): this { return this; } + hide(): void {} +} + +export const Platform = { isDesktopApp: false, isMobile: false }; + +export class FileSystemAdapter { + getBasePath(): string { return '/e2e/fake-vault'; } +} diff --git a/e2e/suites/sync-manager.e2e.test.ts b/e2e/suites/sync-manager.e2e.test.ts new file mode 100644 index 0000000..4416511 --- /dev/null +++ b/e2e/suites/sync-manager.e2e.test.ts @@ -0,0 +1,256 @@ +import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; +import { randomBytes } from 'node:crypto'; +import { SyncManager } from '../../src/logic/sync-manager'; +import { SyncPlanModal, SyncPlanDirection } from '../../src/ui/SyncPlanModal'; +import { SyncConflictModal } from '../../src/ui/SyncConflictModal'; +// `import type` deliberately, not a value import: src/settings.ts also +// exports settings-tab UI (GitLabSyncSettingTab -> FolderSuggest -> +// AbstractInputSuggest etc.) which pulls in far more of `obsidian` than this +// suite's minimal shim provides. A type-only import is erased entirely, so +// none of that module ever loads. +import type { GitLabFilesPushSettings } from '../../src/settings'; +import type { TFile as ObsidianTFile } from 'obsidian'; +import { FakeVault, fakeApp } from '../shim/fake-vault'; +import { TFile } from '../shim/obsidian-request-url'; +import { currentProvider, timeouts } from '../config/env'; +import { GiteaE2EAdapter } from '../providers/gitea-adapter'; +import { GitHubE2EAdapter } from '../providers/github-adapter'; +import { GitLabE2EAdapter } from '../providers/gitlab-adapter'; +import type { ProviderE2EAdapter, ProvisionedProvider } from '../providers/provider-adapter'; +import type { RemoteVerifier } from '../verifier/verifier-contract'; + +// Every push/pull SyncManager does shows a plan-review modal first; bare +// vi.mock (automock) + a per-suite auto-confirm implementation is the same +// pattern tests/logic/sync-manager.test.ts uses for unit tests. Conflict +// modal is left as the bare automock default (does nothing, never invokes +// onChoose) -- that's the real production behavior too: pushFile/pullFile +// return before the conflict modal resolves, so a bare mock is already +// correct, not a simplification of what's being tested. +vi.mock('../../src/ui/SyncPlanModal'); +vi.mock('../../src/ui/SyncConflictModal'); + +interface AdapterWithVerifier extends ProvisionedProvider { + verifier: RemoteVerifier; +} + +function adapterFor(provider: string): ProviderE2EAdapter { + if (provider === 'github') return new GitHubE2EAdapter(); + if (provider === 'gitlab') return new GitLabE2EAdapter(); + return new GiteaE2EAdapter(); +} + +/** + * The E2E `obsidian` shim's `TFile` (e2e/shim/obsidian-request-url.ts) is a + * separate, minimal class from the real `obsidian` package's `TFile` type + * that `SyncManager`'s public methods are typed against -- vitest's runtime + * module alias makes them the same *value* when this suite actually runs, + * but `tsc` type-checks against the real `obsidian` .d.ts regardless of that + * runtime alias, so passing the shim class straight into e.g. `pushFile` + * needs this cast to satisfy the type checker. + */ +function asTFile(path: string): ObsidianTFile { + return new TFile(path) as unknown as ObsidianTFile; +} + +function makeSettings(branch: string): GitLabFilesPushSettings { + return { + serviceType: 'gitea', + gitlabToken: '', gitlabBaseUrl: '', projectId: '', + githubToken: '', githubOwner: '', githubRepo: '', + giteaToken: '', giteaBaseUrl: '', giteaOwner: '', giteaRepo: '', + branch, + syncMetadata: {}, + rootPath: '', + vaultFolder: '', + symlinkHandling: 'skip', + ignorePatterns: '', + lastSeenVersion: '', + bannerDismissedVersion: '', + language: 'system', + autoRefreshOnStartup: true, + }; +} + +/** + * Real SyncManager + real production provider service (see e2e/providers/), + * driven against whichever provider `E2E_PROVIDER` selects -- the same + * adapter/verifier/provisioner the contract suites use, so this suite adds + * no provider-specific logic of its own (see e2e/verifier/verifier-contract.ts + * for what "independent verification" means here). Only the Obsidian + * filesystem boundary is faked (e2e/shim/fake-vault.ts); everything else is + * the real code path. + */ +describe('SyncManager E2E', () => { + const provider = currentProvider(); + const adapter = adapterFor(provider); + let ctx: AdapterWithVerifier; + const runId = randomBytes(4).toString('hex'); + const path = (name: string) => `e2e-sync-${runId}/${name}`; + + beforeAll(async () => { + ctx = (await adapter.provision()) as AdapterWithVerifier; + }, timeouts.containerReadyMs + 30_000); + + afterAll(async () => { + await adapter.teardown(ctx); + }); + + function newManager(vault: FakeVault, settings: GitLabFilesPushSettings): SyncManager { + vi.mocked(SyncPlanModal).mockImplementation(function ( + this: SyncPlanModal, _app: unknown, _plan: unknown, _direction: SyncPlanDirection, onConfirm: () => void + ) { + onConfirm(); + return this; + } as never); + return new SyncManager(fakeApp(vault), ctx.service, settings, undefined, () => false); + } + + it('pushes a new local file, verified independently of the service', async () => { + const filePath = path('new-file.md'); + const vault = new FakeVault(); + vault.writeLocal(filePath, '# local content'); + const settings = makeSettings(ctx.branch); + const manager = newManager(vault, settings); + + const result = await manager.pushFile(filePath); + + expect(result?.sha).toBeTruthy(); + const remote = await ctx.verifier.getFile(filePath, ctx.branch); + expect(remote?.content).toBe('# local content'); + expect(remote?.sha).toBe(result?.sha); + expect(settings.syncMetadata[filePath]?.lastSyncedSha).toBe(remote?.sha); + }); + + it('does not create a remote mutation when pushing an unchanged file', async () => { + const filePath = path('unchanged.md'); + const vault = new FakeVault(); + vault.writeLocal(filePath, 'steady state'); + const settings = makeSettings(ctx.branch); + const manager = newManager(vault, settings); + await manager.pushFile(filePath); + + const shasBefore = await ctx.verifier.listCommitShas(ctx.branch); + const result = await manager.pushFile(filePath); + const shasAfter = await ctx.verifier.listCommitShas(ctx.branch); + + expect(result?.sha).toBeTruthy(); + expect(shasAfter[0]).toBe(shasBefore[0]); + }); + + it('pulls a remote update into the local vault', async () => { + const filePath = path('to-pull.md'); + // Seed the remote directly (not via SyncManager/pullFile), so this + // vault's SyncManager has no syncMetadata baseline for the path yet -- + // e.g. the file was already in the vault before sync was ever run for + // it. That's what makes this a plain pull rather than a conflict: see + // sync-manager.ts's pull conflict check, which only fires when a prior + // lastSyncedSha exists and no longer matches the remote (exercised by + // the "conflict protection" test below, which does establish a + // baseline first). + await ctx.service.pushFile(filePath, 'v1', ctx.branch, 'e2e: seed remote file'); + const vault = new FakeVault(); + vault.writeLocal(filePath, 'v1'); + const settings = makeSettings(ctx.branch); + const manager = newManager(vault, settings); + + // Remote changes out from under the vault -- via the real production + // service, same as another client pushing, not via SyncManager. + const remoteBefore = await ctx.verifier.getFile(filePath, ctx.branch); + await ctx.service.pushFile(filePath, 'v2 from another client', ctx.branch, 'e2e: simulate remote update', remoteBefore?.sha); + + await manager.pullFile(filePath); + + expect(await vault.adapter.read(filePath)).toBe('v2 from another client'); + const remoteAfter = await ctx.verifier.getFile(filePath, ctx.branch); + expect(settings.syncMetadata[filePath]?.lastSyncedSha).toBe(remoteAfter?.sha); + }); + + it('does not overwrite the remote or falsely mark synced when both sides changed', async () => { + const filePath = path('conflict.md'); + const vault = new FakeVault(); + vault.writeLocal(filePath, 'baseline'); + const settings = makeSettings(ctx.branch); + const manager = newManager(vault, settings); + await manager.pushFile(filePath); + const baselineMeta = settings.syncMetadata[filePath]; + + // Diverge both sides from the synced baseline. + vault.writeLocal(filePath, 'local edit'); + const remoteBaseline = await ctx.verifier.getFile(filePath, ctx.branch); + await ctx.service.pushFile(filePath, 'remote edit', ctx.branch, 'e2e: diverge remote', remoteBaseline?.sha); + + const conflictCallsBefore = vi.mocked(SyncConflictModal).mock.calls.length; + const result = await manager.pushFile(filePath); + + expect(result).toBeUndefined(); + expect(vi.mocked(SyncConflictModal).mock.calls.length).toBe(conflictCallsBefore + 1); + const remoteAfter = await ctx.verifier.getFile(filePath, ctx.branch); + expect(remoteAfter?.content).toBe('remote edit'); + expect(settings.syncMetadata[filePath]).toEqual(baselineMeta); + }); + + it('renames/moves a file in exactly one commit, verified independently of the service', async () => { + const oldPath = path('rename/old.md'); + const newPath = path('rename/new.md'); + const vault = new FakeVault(); + vault.writeLocal(oldPath, 'move me'); + const settings = makeSettings(ctx.branch); + const manager = newManager(vault, settings); + await manager.pushFile(oldPath); + + vault.renameLocal(oldPath, newPath); + await manager.trackRename(newPath, oldPath); + const shasBefore = await ctx.verifier.listCommitShas(ctx.branch); + + // Rename detection only runs off a real TFile (sync-manager.ts checks + // `!isString && fileOrPath instanceof TFile` before consulting + // `renamedFrom`) -- a plain path string, as every other scenario in + // this suite uses, always takes the plain-push branch instead, same + // as it does in production when the caller doesn't have a TFile handy. + await manager.pushFile(asTFile(newPath)); + + expect(await ctx.verifier.fileMissing(oldPath, ctx.branch)).toBe(true); + const remote = await ctx.verifier.getFile(newPath, ctx.branch); + expect(remote?.content).toBe('move me'); + const shasAfter = await ctx.verifier.listCommitShas(ctx.branch); + expect(shasAfter.length).toBe(shasBefore.length + 1); + }); + + it('deletes a file via the real service, verified independently', async () => { + // Deletion isn't a SyncManager method -- src/ui/SyncStatusView.ts calls + // gitService.deleteFile directly, so this reproduces that real path. + const filePath = path('to-delete.md'); + const vault = new FakeVault(); + vault.writeLocal(filePath, 'delete me'); + const settings = makeSettings(ctx.branch); + const manager = newManager(vault, settings); + await manager.pushFile(filePath); + expect(await ctx.verifier.fileMissing(filePath, ctx.branch)).toBe(false); + + await ctx.service.deleteFile(filePath, ctx.branch, 'e2e: delete file'); + await manager.clearMetadata(filePath); + + expect(await ctx.verifier.fileMissing(filePath, ctx.branch)).toBe(true); + expect(settings.syncMetadata[filePath]).toBeUndefined(); + }); + + it('pushes a batch of local files in exactly one commit, verified independently', async () => { + const paths = [path('batch/a.md'), path('batch/b.md'), path('batch/c.md')]; + const vault = new FakeVault(); + for (const p of paths) vault.writeLocal(p, `content for ${p}`); + const settings = makeSettings(ctx.branch); + const manager = newManager(vault, settings); + const shasBefore = await ctx.verifier.listCommitShas(ctx.branch); + + const results = await manager.pushAllFiles(paths); + + expect(results.success).toBe(paths.length); + expect(results.failed).toBe(0); + for (const p of paths) { + const remote = await ctx.verifier.getFile(p, ctx.branch); + expect(remote?.content).toBe(`content for ${p}`); + } + const shasAfter = await ctx.verifier.listCommitShas(ctx.branch); + expect(shasAfter.length).toBe(shasBefore.length + 1); + }); +}); diff --git a/scripts/run-e2e.mjs b/scripts/run-e2e.mjs index 144ae48..259c7cb 100644 --- a/scripts/run-e2e.mjs +++ b/scripts/run-e2e.mjs @@ -17,9 +17,18 @@ if (!provider) { const passthrough = args.filter((_, i) => i !== providerIndex && i !== providerIndex + 1); +// Runs the provider's own contract suite plus the shared SyncManager suite +// (parametrized by E2E_PROVIDER, see e2e/suites/sync-manager.e2e.test.ts) in +// the same command/container lifecycle, so one `npm run test:e2e` per +// provider covers both without a second npm script or CI step. const result = spawnSync( 'npx', - ['vitest', 'run', '-c', 'vitest.e2e.config.ts', `e2e/suites/${provider}.e2e.test.ts`, ...passthrough], + [ + 'vitest', 'run', '-c', 'vitest.e2e.config.ts', + `e2e/suites/${provider}.e2e.test.ts`, + 'e2e/suites/sync-manager.e2e.test.ts', + ...passthrough, + ], { stdio: 'inherit', env: { ...process.env, E2E_PROVIDER: provider }, From 49dfaf761de1d6cfd8b1f85650c59b3ce87db8ce Mon Sep 17 00:00:00 2001 From: tianyao Date: Fri, 7 Aug 2026 10:35:59 +0000 Subject: [PATCH 2/6] test(e2e): add listCommitShas to the shared verifier contract GitHubVerifier already had a GitHub-specific listCommitShas (used by the symlink/GraphQL regression suite); promotes it to the shared RemoteVerifier contract and implements it for Gitea and GitLab too, so the new cross-provider SyncManager suite can assert "rename/batch landed as exactly one commit" identically for all three providers instead of casting to a provider-specific verifier type. Also adds E2E_KEEP_BRANCH support to all three provisioners' teardown (skip branch deletion / container removal for debugging a failing run) -- called out in the issue's sandbox-lifecycle acceptance criteria but not yet implemented. --- e2e/provision/gitea-provision.ts | 4 ++++ e2e/provision/github-provision.ts | 4 ++++ e2e/provision/gitlab-provision.ts | 4 ++++ e2e/verifier/gitea-verifier.ts | 8 ++++++++ e2e/verifier/github-verifier.ts | 1 - e2e/verifier/gitlab-verifier.ts | 8 ++++++++ e2e/verifier/verifier-contract.ts | 3 +++ 7 files changed, 31 insertions(+), 1 deletion(-) diff --git a/e2e/provision/gitea-provision.ts b/e2e/provision/gitea-provision.ts index fe768f1..0813fb6 100644 --- a/e2e/provision/gitea-provision.ts +++ b/e2e/provision/gitea-provision.ts @@ -128,6 +128,10 @@ async function createSandboxRepo(baseUrl: string, token: string): Promise /** Best-effort cleanup — safe to call even if provisioning only partially completed. */ export async function teardownGitea(env: Pick): Promise { + if (process.env.E2E_KEEP_BRANCH === '1' || process.env.E2E_KEEP_BRANCH === 'true') { + logInfo(`E2E_KEEP_BRANCH set — leaving container ${env.containerName} running for debugging`); + return; + } logInfo(`Removing container ${env.containerName}`); await removeContainer(env.containerName); logInfo(`Removing network ${env.networkName}`); diff --git a/e2e/provision/github-provision.ts b/e2e/provision/github-provision.ts index 956d9b6..e5f0bdd 100644 --- a/e2e/provision/github-provision.ts +++ b/e2e/provision/github-provision.ts @@ -82,6 +82,10 @@ export async function provisionGitHub(): Promise { /** Best-effort cleanup — safe to call even if provisioning only partially completed. */ export async function teardownGitHub(env: GitHubEnvironment): Promise { + if (process.env.E2E_KEEP_BRANCH === '1' || process.env.E2E_KEEP_BRANCH === 'true') { + logInfo(`E2E_KEEP_BRANCH set — leaving run branch ${env.branch} in place for debugging`); + return; + } logInfo(`Removing run branch ${env.branch}`); try { await fetch(`${API_BASE}/repos/${env.owner}/${env.repo}/git/refs/heads/${encodeURIComponent(env.branch)}`, { diff --git a/e2e/provision/gitlab-provision.ts b/e2e/provision/gitlab-provision.ts index bd434d3..d3d52da 100644 --- a/e2e/provision/gitlab-provision.ts +++ b/e2e/provision/gitlab-provision.ts @@ -51,6 +51,10 @@ export async function provisionGitLab(): Promise { /** Best-effort cleanup — deletes the run-specific branch. Must not throw. */ export async function teardownGitLab(env: GitLabEnvironment): Promise { + if (process.env.E2E_KEEP_BRANCH === '1' || process.env.E2E_KEEP_BRANCH === 'true') { + logInfo(`E2E_KEEP_BRANCH set — leaving branch ${env.branch} in place for debugging`); + return; + } try { logInfo(`Removing branch ${env.branch}`); const encodedProjectId = encodeURIComponent(env.projectId); diff --git a/e2e/verifier/gitea-verifier.ts b/e2e/verifier/gitea-verifier.ts index 910d55c..39867a1 100644 --- a/e2e/verifier/gitea-verifier.ts +++ b/e2e/verifier/gitea-verifier.ts @@ -45,4 +45,12 @@ export class GiteaVerifier implements RemoteVerifier { async fileMissing(path: string, ref: string): Promise { return (await this.getFile(path, ref)) === null; } + + async listCommitShas(ref: string, perPage = 30): Promise { + const url = `${this.baseUrl}/api/v1/repos/${this.owner}/${this.repo}/commits?sha=${encodeURIComponent(ref)}&limit=${perPage}`; + const res = await fetch(url, { headers: this.headers() }); + if (!res.ok) throw new Error(`GiteaVerifier.listCommitShas failed: ${res.status} ${await res.text()}`); + const data = await res.json() as Array<{ sha: string }>; + return data.map(item => item.sha); + } } diff --git a/e2e/verifier/github-verifier.ts b/e2e/verifier/github-verifier.ts index ffbbab3..3fc8353 100644 --- a/e2e/verifier/github-verifier.ts +++ b/e2e/verifier/github-verifier.ts @@ -56,7 +56,6 @@ export class GitHubVerifier implements RemoteVerifier { return tree.find(item => item.path === path)?.mode ?? null; } - /** GitHub-specific: commit shas on `ref`, newest first — used to assert a batch/rename landed as exactly one commit. */ async listCommitShas(ref: string, perPage = 30): Promise { const url = `${API_BASE}/repos/${this.owner}/${this.repo}/commits?sha=${encodeURIComponent(ref)}&per_page=${perPage}`; const res = await fetch(url, { headers: this.headers() }); diff --git a/e2e/verifier/gitlab-verifier.ts b/e2e/verifier/gitlab-verifier.ts index f2f17dd..e2c5c37 100644 --- a/e2e/verifier/gitlab-verifier.ts +++ b/e2e/verifier/gitlab-verifier.ts @@ -53,6 +53,14 @@ export class GitLabVerifier implements RemoteVerifier { return (await this.getFile(path, ref)) === null; } + async listCommitShas(ref: string, perPage = 30): Promise { + const url = `${this.baseUrl}/api/v4/projects/${this.encodedProjectId}/repository/commits?ref_name=${encodeURIComponent(ref)}&per_page=${perPage}`; + const res = await fetch(url, { headers: this.headers() }); + if (!res.ok) throw new Error(`GitLabVerifier.listCommitShas failed: ${res.status} ${await res.text()}`); + const data = await res.json() as Array<{ id: string }>; + return data.map(item => item.id); + } + /** * Fetches the file's `last_commit_id` (GitLab's optimistic-locking * revision) directly, independent of GitLabService.getFile. Used by the diff --git a/e2e/verifier/verifier-contract.ts b/e2e/verifier/verifier-contract.ts index 0f6eddb..6ec569c 100644 --- a/e2e/verifier/verifier-contract.ts +++ b/e2e/verifier/verifier-contract.ts @@ -15,4 +15,7 @@ export interface RemoteVerifier { /** True if `path` does not exist at `ref` (used to verify deletes/renames-away). */ fileMissing(path: string, ref: string): Promise; + + /** Commit shas on `ref`, newest first — used to assert a batch/rename/push landed as exactly N new commits, without trusting the service under test's own commit count. */ + listCommitShas(ref: string, perPage?: number): Promise; } From afa142efaea698ab0f2d63ec494c6276948b0443 Mon Sep 17 00:00:00 2001 From: tianyao Date: Fri, 7 Aug 2026 10:40:34 +0000 Subject: [PATCH 3/6] ci(e2e): run provider checks before shared CI and release Adds a provider-e2e matrix job (github/gitlab/gitea) to .github/workflows/ci.yml, per the issue's runner-fleet revision -- runs-on: [self-hosted, linux, x64, 32gb-ram], one matrix instead of three hand-written jobs. - `changes` job (dorny/paths-filter) decides whether provider-e2e should run for a given push/PR without gating the whole workflow by path -- on.push.paths/on.pull_request.paths would have also blocked the release-critical CI job for unrelated changes, which this avoids. - Fork PRs only get the Gitea cell (no repo secrets needed/exposed); internal PRs, pushes to main, workflow_dispatch, and the weekly schedule (Monday 06:00 UTC, API-drift detection) get all three. - scripts/run-e2e-ci.mjs wraps scripts/run-e2e.mjs for CI: sweeps stale gfs-e2e--* branches first (scripts/e2e-sweep-branches.mjs), and turns a missing required credential into a hard failure rather than a silent skip, for any cell the job-level `if:` already decided should run. - e2e-gate aggregates the matrix (if: always(), success/skipped pass through, anything else fails) and CI now needs it, so a real provider regression blocks the shared CI/semantic-release workflow instead of shipping. E2E_GITLAB_PROJECT_ID is read from secrets, not vars, in this workflow -- confirmed via `gh secret list` that it's configured as a secret (unlike E2E_GITHUB_OWNER/REPO, which are plain vars) on this repo. Verified: scripts/run-e2e-ci.mjs and scripts/e2e-sweep-branches.mjs run correctly against Gitea locally (14/14 passing) and fail explicitly (exit 1, no silent skip) when GitHub credentials are absent; workflow YAML validated with js-yaml. Not verified: actual execution on the self-hosted runner fleet or the full e2e-gate -> CI dependency chain in a real workflow run (no self-hosted runner access from this checkout) -- see docs/testing/real-provider-e2e.md's "Known gaps". --- .github/workflows/ci.yml | 121 +++++++++++++++++++++++++++++++++ scripts/e2e-sweep-branches.mjs | 112 ++++++++++++++++++++++++++++++ scripts/run-e2e-ci.mjs | 48 +++++++++++++ 3 files changed, 281 insertions(+) create mode 100644 scripts/e2e-sweep-branches.mjs create mode 100644 scripts/run-e2e-ci.mjs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index da48184..b21f6ea 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,9 +12,130 @@ on: branches: [main, master, '**'] pull_request: types: [opened, synchronize, reopened] + workflow_dispatch: + inputs: + provider: + description: 'Provider(s) to run (github, gitlab, gitea, or all)' + default: 'all' + keep_branch: + description: 'Keep the E2E branch/container after the run for debugging' + type: boolean + default: false + schedule: + # Weekly API-drift check: same real-provider suites, no code change required to trigger them. + - cron: '0 6 * * 1' jobs: + # `on.push.paths`/`on.pull_request.paths` would gate this *whole* workflow + # file by path -- including the release-critical `CI` job below, which must + # keep running for every push/PR regardless of path. This job instead + # computes a per-job boolean so only `provider-e2e` skips on irrelevant + # changes, while `CI`/`build-artifact` are unaffected. + changes: + name: Detect sync/provider-relevant changes + runs-on: ubuntu-latest + outputs: + e2e-relevant: ${{ steps.filter.outputs.e2e-relevant }} + steps: + - uses: actions/checkout@v6 + - uses: dorny/paths-filter@v3 + id: filter + with: + filters: | + e2e-relevant: + - 'src/services/**' + - 'src/logic/sync-manager.ts' + - 'src/utils/git-blob-sha.ts' + - 'src/utils/path.ts' + - 'src/utils/symlink.ts' + - 'e2e/**' + - 'package.json' + - 'package-lock.json' + - '.github/workflows/ci.yml' + + # Real-provider E2E: one matrix job covering GitHub, GitLab, and Gitea (see + # docs/testing/real-provider-e2e.md). + provider-e2e: + name: E2E / ${{ matrix.provider }} + needs: changes + runs-on: [self-hosted, linux, x64, 32gb-ram] + # Runs when sync/provider-relevant paths changed, or unconditionally on + # workflow_dispatch/schedule/a push to main (main always gets the full + # tier regardless of path, per the issue's CI wiring). Within that: + # internal PRs / pushes to main / manual dispatch / schedule get every + # provider. A fork PR (head repo != base repo) only gets Gitea, which + # needs no repository secrets and can safely run against an untrusted + # fork's code -- GitHub/GitLab need real sandbox credentials that must + # never be exposed to a fork PR's workflow run. + if: >- + ( + needs.changes.outputs.e2e-relevant == 'true' || + github.event_name == 'workflow_dispatch' || + github.event_name == 'schedule' || + github.ref == 'refs/heads/main' + ) && ( + github.event_name != 'pull_request' || + matrix.provider == 'gitea' || + github.event.pull_request.head.repo.full_name == github.repository + ) && ( + github.event_name != 'workflow_dispatch' || + github.event.inputs.provider == 'all' || + github.event.inputs.provider == matrix.provider + ) + strategy: + fail-fast: false + max-parallel: 3 + matrix: + provider: [github, gitlab, gitea] + env: + E2E_GITHUB_OWNER: ${{ vars.E2E_GITHUB_OWNER }} + E2E_GITHUB_REPO: ${{ vars.E2E_GITHUB_REPO }} + E2E_GITHUB_TOKEN: ${{ secrets.E2E_GITHUB_TOKEN }} + # E2E_GITLAB_PROJECT_ID is configured as a repo *secret*, not a + # variable, on firstsun-dev/git-files-sync (confirmed via `gh secret + # list` while wiring this workflow) -- unlike E2E_GITHUB_OWNER/REPO, + # which are plain (non-sensitive) vars. + E2E_GITLAB_PROJECT_ID: ${{ secrets.E2E_GITLAB_PROJECT_ID }} + E2E_GITLAB_TOKEN: ${{ secrets.E2E_GITLAB_TOKEN }} + E2E_KEEP_BRANCH: ${{ github.event.inputs.keep_branch }} + steps: + - uses: actions/checkout@v6 + + - uses: actions/setup-node@v6 + with: + node-version: '22' + cache: npm + + - run: npm ci + + - name: Run provider E2E + run: node scripts/run-e2e-ci.mjs --provider=${{ matrix.provider }} + + # Aggregates the matrix into a single required status so branch protection + # only has to reference one check name (see docs/testing/real-provider-e2e.md + # for the "Gitea required, GitHub/GitLab not required at branch-protection + # level" split -- required-vs-optional per *provider* still comes from the + # matrix `if:` above; this gate only asks "did whatever ran, pass?"). + # `if: always()` so a real provider-e2e failure/cancellation is caught + # here and blocks CI/release, instead of GitHub Actions silently treating + # an upstream failure as "this job never needed to run". + e2e-gate: + name: E2E gate + needs: provider-e2e + if: always() + runs-on: ubuntu-latest + steps: + - name: Check provider-e2e result + run: | + result="${{ needs.provider-e2e.result }}" + echo "provider-e2e result: $result" + if [ "$result" != "success" ] && [ "$result" != "skipped" ]; then + echo "::error::provider-e2e failed or was cancelled ($result) -- blocking CI/release." + exit 1 + fi + CI: + needs: e2e-gate uses: firstsun-dev/.github/.github/workflows/obsidian-plugin-ci.yml@v1 with: plugin-id: "git-file-sync" diff --git a/scripts/e2e-sweep-branches.mjs b/scripts/e2e-sweep-branches.mjs new file mode 100644 index 0000000..18fd769 --- /dev/null +++ b/scripts/e2e-sweep-branches.mjs @@ -0,0 +1,112 @@ +#!/usr/bin/env node +/* + * Best-effort cleanup for leftover `gfs-e2e--*` branches (see + * e2e/namespace.ts) left behind by a crashed/cancelled CI run -- a normal + * run deletes its own branch in teardown (e2e/provision/{github,gitlab}- + * provision.ts). Gitea needs no sweeper: its whole container, not just a + * branch, is torn down in afterAll, and a leftover container is cleaned up + * by the next run reusing the same run-specific container name. + * + * Never throws and never fails its own process: sweeping is opportunistic + * housekeeping run before the required E2E gate (scripts/run-e2e-ci.mjs), + * not part of it. Missing credentials mean "nothing to sweep here", not an + * error -- run-e2e-ci.mjs is what turns missing *required* credentials into + * an explicit failure. + */ + +const MAX_AGE_MS = 24 * 60 * 60 * 1000; +const BRANCH_PREFIX = (provider) => `gfs-e2e-${provider}-`; + +function log(message) { + console.log(`[e2e-sweep] ${message}`); +} + +async function sweepGitHub() { + const owner = process.env.E2E_GITHUB_OWNER; + const repo = process.env.E2E_GITHUB_REPO; + const token = process.env.E2E_GITHUB_TOKEN; + if (!owner || !repo || !token) { + log('github: no credentials configured, skipping'); + return; + } + const headers = { + Authorization: `Bearer ${token}`, + Accept: 'application/vnd.github+json', + 'X-GitHub-Api-Version': '2022-11-28', + }; + const prefix = BRANCH_PREFIX('github'); + + const listRes = await fetch(`https://api.github.com/repos/${owner}/${repo}/branches?per_page=100`, { headers }); + if (!listRes.ok) { + log(`github: failed to list branches (${listRes.status}), skipping`); + return; + } + const branches = await listRes.json(); + + for (const branch of branches) { + if (!branch.name?.startsWith(prefix)) continue; + const commitRes = await fetch(`https://api.github.com/repos/${owner}/${repo}/commits/${branch.commit.sha}`, { headers }); + if (!commitRes.ok) continue; + const commit = await commitRes.json(); + const committedAt = new Date(commit.commit?.committer?.date ?? 0).getTime(); + if (Date.now() - committedAt < MAX_AGE_MS) continue; + + log(`github: deleting stale branch ${branch.name}`); + await fetch(`https://api.github.com/repos/${owner}/${repo}/git/refs/heads/${encodeURIComponent(branch.name)}`, { + method: 'DELETE', + headers, + }).catch(() => {}); + } +} + +async function sweepGitLab() { + const baseUrl = process.env.E2E_GITLAB_BASE_URL ?? 'https://gitlab.com'; + const projectId = process.env.E2E_GITLAB_PROJECT_ID; + const token = process.env.E2E_GITLAB_TOKEN; + if (!projectId || !token) { + log('gitlab: no credentials configured, skipping'); + return; + } + const headers = { 'PRIVATE-TOKEN': token }; + const encodedProjectId = encodeURIComponent(projectId); + const prefix = BRANCH_PREFIX('gitlab'); + + const listRes = await fetch(`${baseUrl}/api/v4/projects/${encodedProjectId}/repository/branches?per_page=100`, { headers }); + if (!listRes.ok) { + log(`gitlab: failed to list branches (${listRes.status}), skipping`); + return; + } + const branches = await listRes.json(); + + for (const branch of branches) { + if (!branch.name?.startsWith(prefix)) continue; + const committedAt = new Date(branch.commit?.committed_date ?? 0).getTime(); + if (Date.now() - committedAt < MAX_AGE_MS) continue; + + log(`gitlab: deleting stale branch ${branch.name}`); + await fetch(`${baseUrl}/api/v4/projects/${encodedProjectId}/repository/branches/${encodeURIComponent(branch.name)}`, { + method: 'DELETE', + headers, + }).catch(() => {}); + } +} + +async function main() { + const providerArg = process.argv.find((arg) => arg.startsWith('--provider=')); + const provider = providerArg?.split('=')[1]; + + const sweeps = { github: sweepGitHub, gitlab: sweepGitLab }; + const toRun = provider ? [provider] : Object.keys(sweeps); + + for (const name of toRun) { + const sweep = sweeps[name]; + if (!sweep) continue; // gitea: no branch sweeper needed, see header comment + try { + await sweep(); + } catch (e) { + log(`${name}: sweep failed, ignoring (best-effort): ${e instanceof Error ? e.message : String(e)}`); + } + } +} + +await main(); diff --git a/scripts/run-e2e-ci.mjs b/scripts/run-e2e-ci.mjs new file mode 100644 index 0000000..916cdad --- /dev/null +++ b/scripts/run-e2e-ci.mjs @@ -0,0 +1,48 @@ +#!/usr/bin/env node +/* + * CI entry point for one `provider-e2e` matrix cell (see + * .github/workflows/ci.yml). Thin wrapper around `scripts/run-e2e.mjs` + * (the same command used locally) adding the two things only CI needs: + * + * 1. Sweep stale `gfs-e2e--*` branches first (scripts/e2e-sweep- + * branches.mjs), so a crashed/cancelled prior run's leftover branch + * doesn't linger indefinitely in the sandbox repo/project. + * 2. Fail loudly, not silently, when credentials are missing. + * + * Whether this provider is *supposed* to run at all for the current event + * (e.g. a fork PR only getting Gitea) is decided by the job-level `if:` in + * ci.yml -- by the time this script runs, the workflow has already decided + * this cell should execute, so missing credentials here always means + * something is actually broken (an unset repo secret/variable), never "this + * event legitimately has no credentials". A missing required secret must be + * an explicit failure, never a silent skip that reports green. + */ +import { spawnSync } from 'node:child_process'; + +const providerArg = process.argv.find((arg) => arg.startsWith('--provider=')); +const provider = providerArg?.split('=')[1]; + +if (!provider) { + console.error('Usage: node scripts/run-e2e-ci.mjs --provider='); + process.exit(1); +} + +const REQUIRED_ENV = { + github: ['E2E_GITHUB_OWNER', 'E2E_GITHUB_REPO', 'E2E_GITHUB_TOKEN'], + gitlab: ['E2E_GITLAB_PROJECT_ID', 'E2E_GITLAB_TOKEN'], + gitea: [], // provisioned entirely inside the job via Docker; no repo secrets needed +}; + +const missing = (REQUIRED_ENV[provider] ?? []).filter((name) => !process.env[name]); +if (missing.length > 0) { + console.error(`::error::provider-e2e/${provider}: missing required credential(s): ${missing.join(', ')}`); + process.exit(1); +} + +function run(command, args) { + const result = spawnSync(command, args, { stdio: 'inherit' }); + if (result.status !== 0) process.exit(result.status ?? 1); +} + +run('node', ['scripts/e2e-sweep-branches.mjs', `--provider=${provider}`]); +run('node', ['scripts/run-e2e.mjs', '--provider', provider]); From 60e2d2cd6ae99d004b0ae58a3bf9fa1afe30b33a Mon Sep 17 00:00:00 2001 From: tianyao Date: Fri, 7 Aug 2026 10:40:56 +0000 Subject: [PATCH 4/6] docs(e2e): document real-provider E2E setup, CI, and troubleshooting Consolidates local setup, required secrets/vars (cross-checked against what's actually configured on firstsun-dev/git-files-sync via `gh secret list`/`gh variable list`, not just what the issue originally proposed), CI wiring, fork/secrets behavior, release gating, and cleanup/ troubleshooting into one operational doc -- docs/test/github-e2e-plan.md (agent 02) stays as the GitHub-specific implementation notes; this is the cross-provider operational reference. Explicitly lists what's unverified from this environment (GitHub/GitLab SyncManager E2E execution, self-hosted runner behavior, branch-protection required-check setup) rather than leaving it implicit. --- docs/testing/real-provider-e2e.md | 122 ++++++++++++++++++++++++++++++ 1 file changed, 122 insertions(+) create mode 100644 docs/testing/real-provider-e2e.md diff --git a/docs/testing/real-provider-e2e.md b/docs/testing/real-provider-e2e.md new file mode 100644 index 0000000..0fe24a1 --- /dev/null +++ b/docs/testing/real-provider-e2e.md @@ -0,0 +1,122 @@ +# Real-provider E2E + +Issue #57. Real `SyncManager`/`GitHubService`/`GitLabService`/`GiteaService` code run +against real GitHub, GitLab, and Gitea servers, with every remote assertion made through an +independent verifier (raw REST calls, never the service under test reading back its own +write). See `e2e/` for the harness itself: + +- `e2e/providers/` — one adapter per provider (`provision()` -> real, already-configured + `GitServiceInterface`; `teardown()`). +- `e2e/provision/` — GitHub/GitLab: validates credentials against a dedicated sandbox + repo/project and creates a run-specific branch. Gitea: provisions a pinned Docker container + from scratch. +- `e2e/verifier/` — one `RemoteVerifier` per provider, raw API calls only. +- `e2e/suites/{github,gitlab,gitea}.e2e.test.ts` — provider contract suites (create/read/ + update/delete/batch/rename, plus provider-specific regressions). +- `e2e/suites/sync-manager.e2e.test.ts` — one suite, parametrized by `E2E_PROVIDER`, covering + `SyncManager` itself (push/pull/conflict/rename/delete/batch) against a real provider with an + in-memory fake Vault (`e2e/shim/fake-vault.ts`) standing in for the Obsidian filesystem + boundary — see that file's header comment for why the vault is the only thing faked. + +## Running locally + +```sh +npm run test:e2e -- --provider gitea # no credentials needed, runs a real Gitea in Docker +npm run test:e2e -- --provider github # needs E2E_GITHUB_* below +npm run test:e2e -- --provider gitlab # needs E2E_GITLAB_* below +``` + +Each command runs that provider's contract suite *and* the SyncManager suite in one process +(`scripts/run-e2e.mjs`). Export credentials in your shell before running (there is no +`.env`-style file loader in this harness — plain `process.env`, matching `e2e/config/env.ts`): + +| Var | Required for | Notes | +|---|---|---| +| `E2E_GITHUB_OWNER` | github | e.g. `firstsun-dev` | +| `E2E_GITHUB_REPO` | github | dedicated sandbox repo — **never** a real user's repo | +| `E2E_GITHUB_TOKEN` | github | fine-grained PAT, scoped to that one repo, Contents: Read and write | +| `E2E_GITHUB_BASE_BRANCH` | github (optional) | defaults to `main` | +| `E2E_GITLAB_PROJECT_ID` | gitlab | dedicated sandbox project | +| `E2E_GITLAB_TOKEN` | gitlab | token with `api` scope on that project — `write_repository` alone is not enough, the verifier and branch setup use REST endpoints outside its coverage | +| `E2E_GITLAB_BASE_URL` | gitlab (optional) | defaults to `https://gitlab.com` | +| `E2E_KEEP_BRANCH` | any (optional) | `1`/`true` skips teardown (branch for GitHub/GitLab, container for Gitea) so you can inspect a failing run | + +Gitea needs Docker locally and nothing else — see `e2e/provision/gitea-provision.ts`. + +## CI + +`.github/workflows/ci.yml` runs a `provider-e2e` matrix job (`github`, `gitlab`, `gitea`) via +`scripts/run-e2e-ci.mjs`, gated on relevant paths (`src/services/**`, +`src/logic/sync-manager.ts`, `e2e/**`, etc. — computed by the `changes` job, since GitHub +Actions' own `on.*.paths` would gate the *entire* workflow file, including the always-must-run +`CI`/release job). It always runs in full on `workflow_dispatch`, `schedule` (weekly, Monday +06:00 UTC, for API-drift detection), and pushes to `main`. + +**Secrets/variables** (repo-level, `firstsun-dev/git-files-sync`; confirmed already configured +via `gh secret list` / `gh variable list` while wiring this workflow): + +| Name | Kind | +|---|---| +| `E2E_GITHUB_TOKEN` | secret | +| `E2E_GITHUB_OWNER` | variable | +| `E2E_GITHUB_REPO` | variable | +| `E2E_GITLAB_PROJECT_ID` | secret (not a variable — it's treated as sensitive here) | +| `E2E_GITLAB_TOKEN` | secret | + +**Fork PRs** only run the Gitea cell (`provider-e2e`'s job `if:` checks +`github.event.pull_request.head.repo.full_name == github.repository`) — GitHub/GitLab need +real credentials that must never be exposed to an untrusted fork's workflow run. Gitea needs no +repo secrets at all, so it's safe to run unconditionally. + +**Missing credentials are always a hard failure**, never a silent skip, for any cell that +actually runs (`scripts/run-e2e-ci.mjs` checks required env vars up front) — the job-level `if:` +above is what decides whether a cell *should* run for a given event; once it runs, it's expected +to have what it needs. + +## Release gating + +``` +changes -> provider-e2e [github | gitlab | gitea, parallel] -> e2e-gate -> CI (shared workflow, includes semantic-release) +``` + +`e2e-gate` runs with `if: always()` and treats `provider-e2e`'s aggregate result as pass-through +on `success` or `skipped` (the latter covers path-filtered-out runs), and a hard failure on +anything else — so a real provider regression blocks the release instead of shipping and being +caught after the fact. + +**Branch protection** (not something this repo checkout can change — a GitHub repo-settings +change, left for whoever has admin access): add `E2E / gitea` as a required status check. +GitHub/GitLab (`E2E / github`, `E2E / gitlab`) are deliberately **not** required at the +branch-protection level, so a fork PR (which only runs Gitea) is never wedged by checks it +structurally cannot produce — internal-PR/main-branch release gating still depends on them +through the `e2e-gate`/`CI` job dependency chain above, just not through branch protection. + +## Cleanup / troubleshooting + +- **Stale `gfs-e2e--*` branch** (GitHub/GitLab only — Gitea's whole container is + destroyed in `afterAll`): `scripts/run-e2e-ci.mjs` runs `scripts/e2e-sweep-branches.mjs` + before every CI run, which best-effort deletes any branch of that pattern older than 24h. Run + it manually (`node scripts/e2e-sweep-branches.mjs --provider github`) if you need it sooner. +- **Inspecting a failing run**: set `E2E_KEEP_BRANCH=1` before running so teardown is skipped, + then look at the branch/container directly. Remember to clean it up yourself afterward, or let + the sweeper (GitHub/GitLab) catch it after 24h. +- **Gitea container port/name clashes**: every Docker resource is namespaced per run + (`e2e/namespace.ts`, `gfs-e2e-gitea--` in CI, `gfs-e2e-gitea-local-` + locally), so concurrent runs on the same Docker host don't collide — a leftover container from + an interrupted local run can just be removed manually (`docker rm -f `). +- **`E2E_PROVIDER is not set` error**: the E2E vitest config (`vitest.e2e.config.ts`) refuses to + run directly under `npx vitest` — always go through `npm run test:e2e -- --provider ` (or + `scripts/run-e2e-ci.mjs` in CI), which sets it. + +## Known gaps + +- SyncManager E2E against GitHub/GitLab is written to the same harness as Gitea (no + provider-specific code) but has only been run end-to-end locally against Gitea (Docker, + no external credentials available in that environment) — not yet actually executed against + live GitHub/GitLab sandboxes. Lint/build/typecheck pass for all three. +- The `provider-e2e` matrix job targets `runs-on: [self-hosted, linux, x64, 32gb-ram]` per the + issue's runner-fleet revision; its actual execution on that fleet, and the `e2e-gate` -> + `CI` dependency chain end-to-end in a real workflow run, are unverified from this checkout + (no self-hosted runner access here). +- Branch-protection required-check configuration (`E2E / gitea`) is a manual follow-up for + whoever has admin access to the repo. From 155e55c14fa7a8c19cfaa83da7d27d37605e6cef Mon Sep 17 00:00:00 2001 From: tianyao Date: Fri, 7 Aug 2026 11:34:43 +0000 Subject: [PATCH 5/6] fix(ci): move matrix-dependent E2E gating out of job-level if GitHub Actions rejects the workflow file with 'Unrecognized named-value: matrix' -- job-level `if:` has no access to the `matrix` context, only step-level `if:` does. The provider-e2e job's `if:` referenced matrix.provider to skip GitHub/GitLab legs on fork PRs and to filter by workflow_dispatch input, which is invalid. Fix: keep only the non-matrix-dependent condition (path-relevance/dispatch/ schedule/main) on the job's own if:, and move the matrix-dependent part into a new 'Determine whether this provider leg should run' step that gates every subsequent step via its output. A gated-off leg's steps are all skipped without failing, so the job (and therefore the matrix as a whole, for the e2e-gate aggregation) still reports success -- same external behavior as originally intended, just relocated to where GitHub Actions actually allows matrix to be read. Also corrected docs/testing/real-provider-e2e.md and the comment in scripts/run-e2e-ci.mjs that described the now-nonexistent job-level if mechanism. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/ci.yml | 61 ++++++++++++++++++++----------- docs/testing/real-provider-e2e.md | 5 ++- scripts/run-e2e-ci.mjs | 6 ++- 3 files changed, 47 insertions(+), 25 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b21f6ea..63b0d28 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -61,27 +61,18 @@ jobs: runs-on: [self-hosted, linux, x64, 32gb-ram] # Runs when sync/provider-relevant paths changed, or unconditionally on # workflow_dispatch/schedule/a push to main (main always gets the full - # tier regardless of path, per the issue's CI wiring). Within that: - # internal PRs / pushes to main / manual dispatch / schedule get every - # provider. A fork PR (head repo != base repo) only gets Gitea, which - # needs no repository secrets and can safely run against an untrusted - # fork's code -- GitHub/GitLab need real sandbox credentials that must - # never be exposed to a fork PR's workflow run. + # tier regardless of path, per the issue's CI wiring). The per-provider + # part of the gating (internal PRs/main/dispatch/schedule get every + # provider; a fork PR only gets Gitea) can't live here: job-level `if:` + # has no access to the `matrix` context (GitHub Actions error + # "Unrecognized named-value: 'matrix'" if you try) -- only step-level + # `if:` can see it. That part is done by the "Determine whether this + # provider leg should run" step below instead, gating every later step. if: >- - ( - needs.changes.outputs.e2e-relevant == 'true' || - github.event_name == 'workflow_dispatch' || - github.event_name == 'schedule' || - github.ref == 'refs/heads/main' - ) && ( - github.event_name != 'pull_request' || - matrix.provider == 'gitea' || - github.event.pull_request.head.repo.full_name == github.repository - ) && ( - github.event_name != 'workflow_dispatch' || - github.event.inputs.provider == 'all' || - github.event.inputs.provider == matrix.provider - ) + needs.changes.outputs.e2e-relevant == 'true' || + github.event_name == 'workflow_dispatch' || + github.event_name == 'schedule' || + github.ref == 'refs/heads/main' strategy: fail-fast: false max-parallel: 3 @@ -99,23 +90,51 @@ jobs: E2E_GITLAB_TOKEN: ${{ secrets.E2E_GITLAB_TOKEN }} E2E_KEEP_BRANCH: ${{ github.event.inputs.keep_branch }} steps: + # Per-provider gate (needs `matrix`, so it runs as a step, not the job-level + # `if:` above -- see the comment on that `if:` for why). A fork PR (head repo + # != base repo) only gets Gitea, which needs no repository secrets and can + # safely run against an untrusted fork's code; GitHub/GitLab need real sandbox + # credentials that must never be exposed to a fork PR's workflow run. All + # other events/providers run. + - name: Determine whether this provider leg should run + id: gate + run: | + run=true + if [ "${{ github.event_name }}" = "pull_request" ] \ + && [ "${{ matrix.provider }}" != "gitea" ] \ + && [ "${{ github.event.pull_request.head.repo.full_name }}" != "${{ github.repository }}" ]; then + run=false + fi + if [ "${{ github.event_name }}" = "workflow_dispatch" ] \ + && [ "${{ github.event.inputs.provider }}" != "all" ] \ + && [ "${{ github.event.inputs.provider }}" != "${{ matrix.provider }}" ]; then + run=false + fi + echo "run=$run" >> "$GITHUB_OUTPUT" + - uses: actions/checkout@v6 + if: steps.gate.outputs.run == 'true' - uses: actions/setup-node@v6 + if: steps.gate.outputs.run == 'true' with: node-version: '22' cache: npm - run: npm ci + if: steps.gate.outputs.run == 'true' - name: Run provider E2E + if: steps.gate.outputs.run == 'true' run: node scripts/run-e2e-ci.mjs --provider=${{ matrix.provider }} # Aggregates the matrix into a single required status so branch protection # only has to reference one check name (see docs/testing/real-provider-e2e.md # for the "Gitea required, GitHub/GitLab not required at branch-protection # level" split -- required-vs-optional per *provider* still comes from the - # matrix `if:` above; this gate only asks "did whatever ran, pass?"). + # "Determine whether this provider leg should run" step above; this gate + # only asks "did whatever ran, pass?"). A gated-off leg's steps are all + # skipped without failing the job, so it still reports "success" here. # `if: always()` so a real provider-e2e failure/cancellation is caught # here and blocks CI/release, instead of GitHub Actions silently treating # an upstream failure as "this job never needed to run". diff --git a/docs/testing/real-provider-e2e.md b/docs/testing/real-provider-e2e.md index 0fe24a1..1ad77d9 100644 --- a/docs/testing/real-provider-e2e.md +++ b/docs/testing/real-provider-e2e.md @@ -63,8 +63,9 @@ via `gh secret list` / `gh variable list` while wiring this workflow): | `E2E_GITLAB_PROJECT_ID` | secret (not a variable — it's treated as sensitive here) | | `E2E_GITLAB_TOKEN` | secret | -**Fork PRs** only run the Gitea cell (`provider-e2e`'s job `if:` checks -`github.event.pull_request.head.repo.full_name == github.repository`) — GitHub/GitLab need +**Fork PRs** only run the Gitea cell (checked in the `Determine whether this provider leg should +run` step — GitHub Actions job-level `if:` can't reference the `matrix` context, so this can't +live on the job itself; it gates every later step instead) — GitHub/GitLab need real credentials that must never be exposed to an untrusted fork's workflow run. Gitea needs no repo secrets at all, so it's safe to run unconditionally. diff --git a/scripts/run-e2e-ci.mjs b/scripts/run-e2e-ci.mjs index 916cdad..b17aa37 100644 --- a/scripts/run-e2e-ci.mjs +++ b/scripts/run-e2e-ci.mjs @@ -10,8 +10,10 @@ * 2. Fail loudly, not silently, when credentials are missing. * * Whether this provider is *supposed* to run at all for the current event - * (e.g. a fork PR only getting Gitea) is decided by the job-level `if:` in - * ci.yml -- by the time this script runs, the workflow has already decided + * (e.g. a fork PR only getting Gitea) is decided by the "Determine whether + * this provider leg should run" step in ci.yml (job-level `if:` can't see + * the `matrix` context, so that gate has to be a step, not the job's own + * `if:`) -- by the time this script runs, that gate has already decided * this cell should execute, so missing credentials here always means * something is actually broken (an unset repo secret/variable), never "this * event legitimately has no credentials". A missing required secret must be From 147736e7a11ac053d8e220c6beadd191fae25d03 Mon Sep 17 00:00:00 2001 From: tianyao Date: Fri, 7 Aug 2026 11:57:29 +0000 Subject: [PATCH 6/6] fix(e2e): fix commit-count pagination bug, guard teardown, log gitea failures Investigated the three provider-e2e CI failures from the run right after the matrix if: fix: - github: sync-manager.e2e.test.ts's rename/batch tests asserted listCommitShas(branch).length grew by exactly 1, but the sandbox repo's main already has 47 commits -- past the API's default page size (30) -- so both the 'before' and 'after' calls silently cap at 30 and the assertion ("expected 30 to be 31") fails deterministically, not flakily. Confirmed by querying the real sandbox repo directly. Fixed by comparing HEAD-before against the two newest commits after (listCommitShas(ref, 2)) instead of full-list length -- exact regardless of total history depth, matching the pattern github.e2e.test.ts already uses correctly. Verified against the real sandbox: both previously-failing assertions now pass. - gitea: Docker container never answered its healthcheck within 60s on the self-hosted runner. Reproduced locally with the same code (Docker available here) and it passed cleanly in ~17s -- not a code bug, looks like a runner-side Docker/network blip. Can't fix infra flakiness from here, so instead made the next occurrence self-diagnosing: capture the container's own stdout/stderr on a readiness timeout and attach it to the thrown error (docker.ts: containerLogsAllowFailure; wired into gitea-provision.ts's catch), so a future CI failure shows *why* Gitea never came up instead of just "fetch failed". - Same teardown TypeError as already fixed in gitlab.e2e.test.ts (afterAll running adapter.teardown(ctx) with ctx still undefined when beforeAll fails) also existed in gitea.e2e.test.ts and sync-manager.e2e.test.ts -- applied the same 'if (ctx)' guard to both. Co-Authored-By: Claude Sonnet 5 --- e2e/provision/docker.ts | 14 ++++++++++++++ e2e/provision/gitea-provision.ts | 12 +++++++++--- e2e/suites/gitea.e2e.test.ts | 4 +++- e2e/suites/sync-manager.e2e.test.ts | 29 ++++++++++++++++++++++------- 4 files changed, 48 insertions(+), 11 deletions(-) diff --git a/e2e/provision/docker.ts b/e2e/provision/docker.ts index e860a84..5ee0d9a 100644 --- a/e2e/provision/docker.ts +++ b/e2e/provision/docker.ts @@ -31,6 +31,20 @@ export async function removeContainer(name: string): Promise { await dockerAllowFailure(['rm', '-f', name]); } +/** Best-effort: the container's own stdout/stderr, for diagnosing a readiness + * timeout (e.g. a slow/failed startup) directly from CI output instead of + * needing shell access to the runner. Never throws. `docker logs` writes the + * container's stdout/stderr to its own stdout/stderr respectively, so both + * are captured and combined, not just stdout. */ +export async function containerLogsAllowFailure(name: string, tailLines = 200): Promise { + try { + const { stdout, stderr } = await execFileAsync('docker', ['logs', '--tail', String(tailLines), name]); + return [stdout, stderr].filter(Boolean).join('\n').trim(); + } catch (e) { + return `(failed to fetch container logs: ${e instanceof Error ? e.message : String(e)})`; + } +} + /** Reads back the dynamic host port Docker assigned for a `-p 0:` mapping. */ export async function hostPortFor(containerName: string, containerPort: number): Promise { const output = await docker(['port', containerName, String(containerPort)]); diff --git a/e2e/provision/gitea-provision.ts b/e2e/provision/gitea-provision.ts index 0813fb6..0487ad9 100644 --- a/e2e/provision/gitea-provision.ts +++ b/e2e/provision/gitea-provision.ts @@ -4,7 +4,7 @@ import { promisify } from 'node:util'; import { runNamespace } from '../namespace'; import { globalSecrets, logInfo } from '../redact'; import { giteaImage, timeouts } from '../config/env'; -import { createNetwork, removeNetwork, removeContainer, hostPortFor, waitUntilReady, docker } from './docker'; +import { createNetwork, removeNetwork, removeContainer, hostPortFor, waitUntilReady, docker, containerLogsAllowFailure } from './docker'; const execFileAsync = promisify(execFile); @@ -86,9 +86,15 @@ export async function provisionGitea(): Promise { return { baseUrl, owner: ADMIN_USERNAME, repo: SANDBOX_REPO, token, containerName, networkName }; } catch (e) { - // Provisioning failed partway through — clean up what we started before rethrowing. + // Provisioning failed partway through. A readiness timeout in particular + // gives no clue *why* Gitea never came up (self-hosted runner Docker/ + // network hiccup vs. a real startup failure) without runner shell access + // -- attach the container's own logs to the error before it's torn down, + // so a future CI failure is diagnosable straight from the job output. + const logs = await containerLogsAllowFailure(containerName); await teardownGitea({ containerName, networkName } as GiteaEnvironment); - throw e; + const message = e instanceof Error ? e.message : String(e); + throw new Error(`${message}\n\n-- gitea container logs (tail) --\n${logs}`); } } diff --git a/e2e/suites/gitea.e2e.test.ts b/e2e/suites/gitea.e2e.test.ts index 5f15117..e453013 100644 --- a/e2e/suites/gitea.e2e.test.ts +++ b/e2e/suites/gitea.e2e.test.ts @@ -18,7 +18,9 @@ describe('GiteaService E2E', () => { }, timeouts.containerReadyMs + 30_000); afterAll(async () => { - await adapter.teardown(ctx); + // Guard against beforeAll failing before ctx is assigned (e.g. Docker/ + // container-readiness failure) — teardown must not throw in that case either. + if (ctx) await adapter.teardown(ctx); }); it('testConnection reports the repo and branch as reachable', async () => { diff --git a/e2e/suites/sync-manager.e2e.test.ts b/e2e/suites/sync-manager.e2e.test.ts index 4416511..6fa9893 100644 --- a/e2e/suites/sync-manager.e2e.test.ts +++ b/e2e/suites/sync-manager.e2e.test.ts @@ -92,7 +92,10 @@ describe('SyncManager E2E', () => { }, timeouts.containerReadyMs + 30_000); afterAll(async () => { - await adapter.teardown(ctx); + // Guard against beforeAll failing before ctx is assigned (e.g. Docker/ + // container-readiness failure, missing credentials) — teardown must not + // throw in that case either. + if (ctx) await adapter.teardown(ctx); }); function newManager(vault: FakeVault, settings: GitLabFilesPushSettings): SyncManager { @@ -200,7 +203,15 @@ describe('SyncManager E2E', () => { vault.renameLocal(oldPath, newPath); await manager.trackRename(newPath, oldPath); - const shasBefore = await ctx.verifier.listCommitShas(ctx.branch); + // Just the current HEAD, not a full list: the sandbox repo's base branch + // already carries pre-existing history (e.g. 47 commits on this GitHub + // sandbox's `main` at the time of writing), so asserting on + // listCommitShas(...).length would silently start failing forever once + // that history exceeds the API's default page size (30) -- the "before" + // and "after" calls both cap at the same page size and stop reflecting + // real growth. Comparing HEAD-before against the two newest commits + // after is exact regardless of total history depth. + const [headBefore] = await ctx.verifier.listCommitShas(ctx.branch, 1); // Rename detection only runs off a real TFile (sync-manager.ts checks // `!isString && fileOrPath instanceof TFile` before consulting @@ -212,8 +223,9 @@ describe('SyncManager E2E', () => { expect(await ctx.verifier.fileMissing(oldPath, ctx.branch)).toBe(true); const remote = await ctx.verifier.getFile(newPath, ctx.branch); expect(remote?.content).toBe('move me'); - const shasAfter = await ctx.verifier.listCommitShas(ctx.branch); - expect(shasAfter.length).toBe(shasBefore.length + 1); + const [headAfter, headAfterParent] = await ctx.verifier.listCommitShas(ctx.branch, 2); + expect(headAfter).not.toBe(headBefore); + expect(headAfterParent).toBe(headBefore); }); it('deletes a file via the real service, verified independently', async () => { @@ -240,7 +252,9 @@ describe('SyncManager E2E', () => { for (const p of paths) vault.writeLocal(p, `content for ${p}`); const settings = makeSettings(ctx.branch); const manager = newManager(vault, settings); - const shasBefore = await ctx.verifier.listCommitShas(ctx.branch); + // See the rename test above for why this compares HEAD-before against + // the two newest commits after, rather than list length. + const [headBefore] = await ctx.verifier.listCommitShas(ctx.branch, 1); const results = await manager.pushAllFiles(paths); @@ -250,7 +264,8 @@ describe('SyncManager E2E', () => { const remote = await ctx.verifier.getFile(p, ctx.branch); expect(remote?.content).toBe(`content for ${p}`); } - const shasAfter = await ctx.verifier.listCommitShas(ctx.branch); - expect(shasAfter.length).toBe(shasBefore.length + 1); + const [headAfter, headAfterParent] = await ctx.verifier.listCommitShas(ctx.branch, 2); + expect(headAfter).not.toBe(headBefore); + expect(headAfterParent).toBe(headBefore); }); });